Add Psalm security checks

[cedric-anne]

Checklist before requesting a review

• I have read the CONTRIBUTING document.
• I have performed a self-review of my code.

Description

This PR add a security scan using Psalm.

I used cpx/cpx to execute the command as the package dependencies are not compatible with some of our dependencies.

[cedric-anne]

For now, I am not able to find a way to suppress false-positive errors.

For instance, I tried to handle the following error:

ERROR: TaintedInclude - src/autoload/legacy-autoloader.php:80:22 - Detected tainted code passed to include or similar (see https://psalm.dev/251)
  $_POST
    <no known location>

  $_POST['itemtype'] - ajax/dropdownMassiveActionField.php:79:35
        ($plug = isPluginItemType($_POST["itemtype"]))

  call to isPluginItemType - ajax/dropdownMassiveActionField.php:79:35
        ($plug = isPluginItemType($_POST["itemtype"]))

  isPluginItemType#1 - src/autoload/misc-functions.php:71:27
function isPluginItemType($classname)

  $classname - src/autoload/misc-functions.php:71:27
function isPluginItemType($classname)

  call to explode - src/autoload/misc-functions.php:80:30
        $tab = explode('\\', $classname, 3);

  explode#2 - src/autoload/misc-functions.php:80:30
        $tab = explode('\\', $classname, 3);

  explode - ../../../home/www-data/.cpx/vimeo/psalm/latest/vendor/vimeo/psalm/stubs/CoreGenericFunctions.phpstub:784:72
function explode(string $separator, string $string, int $limit = -1) : array {}

  $tab - src/autoload/misc-functions.php:80:9
        $tab = explode('\\', $classname, 3);

  $tab[2] - src/autoload/misc-functions.php:83:27
        $plug['class']  = $tab[2];

  $plug - src/autoload/misc-functions.php:83:9
        $plug['class']  = $tab[2];

  ispluginitemtype - src/autoload/misc-functions.php:71:10
function isPluginItemType($classname)

  $plug - src/autoload/legacy-autoloader.php:48:5
    $plug = isPluginItemType($classname);

  $plug['class'] - src/autoload/legacy-autoloader.php:60:21
    $plugin_class = $plug['class'];

  $plugin_class - src/autoload/legacy-autoloader.php:60:5
    $plugin_class = $plug['class'];

  call to strtolower - src/autoload/legacy-autoloader.php:76:108
    $legacy_path          = sprintf('%s/inc/%s.class.php', $plugin_path, str_replace('\\', '/', strtolower($plugin_class)));

  strtolower#1 - src/autoload/legacy-autoloader.php:76:108
    $legacy_path          = sprintf('%s/inc/%s.class.php', $plugin_path, str_replace('\\', '/', strtolower($plugin_class)));

  strtolower - ../../../home/www-data/.cpx/vimeo/psalm/latest/vendor/vimeo/psalm/stubs/CoreGenericFunctions.phpstub:599:39
function strtolower(string $string) : string {}

  call to str_replace - src/autoload/legacy-autoloader.php:76:97
    $legacy_path          = sprintf('%s/inc/%s.class.php', $plugin_path, str_replace('\\', '/', strtolower($plugin_class)));

  str_replace#3 - src/autoload/legacy-autoloader.php:76:97
    $legacy_path          = sprintf('%s/inc/%s.class.php', $plugin_path, str_replace('\\', '/', strtolower($plugin_class)));

  str_replace - ../../../home/www-data/.cpx/vimeo/psalm/latest/vendor/vimeo/psalm/stubs/CoreGenericFunctions.phpstub:1013:10
function str_replace($search, $replace, $subject, &$count = null) {}

  call to sprintf - src/autoload/legacy-autoloader.php:76:74
    $legacy_path          = sprintf('%s/inc/%s.class.php', $plugin_path, str_replace('\\', '/', strtolower($plugin_class)));

  sprintf#3 - src/autoload/legacy-autoloader.php:76:74
    $legacy_path          = sprintf('%s/inc/%s.class.php', $plugin_path, str_replace('\\', '/', strtolower($plugin_class)));

  sprintf - ../../../home/www-data/.cpx/vimeo/psalm/latest/vendor/vimeo/psalm/stubs/CoreGenericFunctions.phpstub:1301:10
function sprintf(string $format, ...$values) {}

  $legacy_path - src/autoload/legacy-autoloader.php:76:5
    $legacy_path          = sprintf('%s/inc/%s.class.php', $plugin_path, str_replace('\\', '/', strtolower($plugin_class)));

Even if I consider that the function is already secure enough (a classname cannot contain a ., so it is not possible to include a file located ouside th GLPI or the corresponding directory), I tried to a security chec and use the @psalm-suppress TaintedInclude PHPDoc tag. I still get the error in the report.

index 0365b72f38..8a96432ee1 100644
--- a/src/autoload/legacy-autoloader.php
+++ b/src/autoload/legacy-autoloader.php
@@ -34,14 +34,17 @@
  */
 
 /**
- * Classes loader
+ * GLPI autoloader for classes that do not follow the PSR-4.
  *
- * @param string $classname : class to load
- *
- * @return void|boolean
+ * @psalm-suppress TaintedInclude
  */
-function glpi_autoload($classname)
+function glpi_autoload(string $classname): void
 {
+    if (preg_match('/\./', $classname) === 1) {
+        // security check, a valid classname is not supposed to contain a `.`
+        return;
+    }
+
     $plug = isPluginItemType($classname);
     if (!$plug) {
         // PSR-4 styled autoloading for classes without namespace
@@ -57,7 +60,7 @@ function glpi_autoload($classname)
     $plugin_class = $plug['class'];
 
     if (!Plugin::isPluginLoaded($plugin_key)) {
-        return false;
+        return;
     }
 
     $plugin_path = null;

[trasher]

As far as I've seen, psalm just requires composer deps; we should save time and resources if we do not build JS, locales etc.

[cedric-anne]

For now, it is not possible to add it on GLPI. It reports too many false positive issues.