Security-focused PHPstan

composer.json

@@ -102,7 +102,6 @@
         "behat/mink": "^1.10",
         "behat/mink-browserkit-driver": "^2.1",
         "behat/mink-selenium2-driver": "^1.6",
-        "mink/webdriver-classic-driver": "^1.1",
         "brianium/paratest": "^6.11",
         "consolidation/robo": "^4",
         "dealerdirect/phpcodesniffer-composer-installer": "^0.7.0",
@@ -113,10 +112,12 @@
         "drupal/potx": "^1.0@alpha",
         "gettext/gettext": "^5.7",
         "mikey179/vfsstream": "^1.6",
+        "mink/webdriver-classic-driver": "^1.1",
         "mpyw/phpunit-patch-serializable-comparison": "^0.0.2",
         "natxet/cssmin": "^3.0",
         "phpspec/prophecy-phpunit": "^2",
         "scssphp/scssphp": "^1.0.0",
+        "spaze/phpstan-disallowed-calls": "^4.6",
         "symfony/browser-kit": "^6.3",
         "symfony/phpunit-bridge": "^5.0",
         "totten/lurkerlite": "^1",

phpstan.neon

@@ -2,18 +2,11 @@ includes:
   - vendor/mglaman/phpstan-drupal/extension.neon
   - vendor/phpstan/phpstan-deprecation-rules/rules.neon
   - phpstan-rules/phpstan-extension.neon
+  - vendor/spaze/phpstan-disallowed-calls/extension.neon
+  - vendor/spaze/phpstan-disallowed-calls/disallowed-dangerous-calls.neon
+  - vendor/spaze/phpstan-disallowed-calls/disallowed-insecure-calls.neon
 parameters:
   level: 6
-  reportUnmatchedIgnoredErrors: false
-  treatPhpDocTypesAsCertain: false
-  scanFiles:
-    - web/core/modules/update/update.compare.inc
-  ignoreErrors:
-    - identifier: missingType.iterableValue
-    - identifier: missingType.generics
-    - '#has no return type specified.#'
-    - '#Access to an undefined property Drupal\\Core\\Field\\FieldItemListInterface::\$value.#'
-    - '#Access to an undefined property Drupal\\Core\\Field\\FieldItemInterface::\$value.#'
   paths:
     - web
     - RoboFile.php
@@ -24,6 +17,7 @@ parameters:
     - '*/tests/fixtures/*.php'
     - 'web/core/*'
     - 'web/sites/default/files/*'
+    # The installer scaffolds these per-project; analysing them just causes noise.
     - 'web/sites/default/*.php'
     - 'web/sites/default/default.settings.php'
     - 'web/sites/default/settings.php (?)'
@@ -32,3 +26,63 @@ parameters:
     - 'web/themes/contrib/*'
     - 'web/libraries/*'
     - 'web/sites/simpletest/*'
+  reportUnmatchedIgnoredErrors: false
+  treatPhpDocTypesAsCertain: false
+  scanFiles:
+    # Update module ships with procedural helpers that Drupal still expects.
+    - web/core/modules/update/update.compare.inc
+  # Keep these ignores in sync with the upstream Drupal level 6 baseline.
+  ignoreErrors:
+    - identifier: missingType.iterableValue
+    - identifier: missingType.generics
+    - '#has no return type specified.#'
+    - '#Access to an undefined property Drupal\\Core\\Field\\FieldItemListInterface::\$value.#'
+    - '#Access to an undefined property Drupal\\Core\\Field\\FieldItemInterface::\$value.#'
+  disallowedFunctionCalls:
+    # Prefer the Symfony Process component for shell execution for better security and DX.
+    -
+      function: 'exec()'
+      message: 'use the Symfony Process component instead (https://symfony.com/doc/current/components/process.html)'
+      allowIn:
+        - robo-components/*
+        - RoboFile.php
+    -
+      function: 'passthru()'
+      message: 'use the Symfony Process component instead (https://symfony.com/doc/current/components/process.html)'
+      allowIn:
+        - robo-components/*
+        - RoboFile.php
+    -
+      function: 'proc_open()'
+      message: 'use the Symfony Process component instead (https://symfony.com/doc/current/components/process.html)'
+      allowIn:
+        - robo-components/*
+        - RoboFile.php
+    -
+      function: 'shell_exec()'
+      message: 'use the Symfony Process component instead (https://symfony.com/doc/current/components/process.html)'
+      allowIn:
+        - robo-components/*
+        - RoboFile.php
+    -
+      function: 'system()'
+      message: 'use the Symfony Process component instead (https://symfony.com/doc/current/components/process.html)'
+      allowIn:
+        - robo-components/*
+        - RoboFile.php
+    -
+      function: 'popen()'
+      message: 'use the Symfony Process component instead (https://symfony.com/doc/current/components/process.html)'
+      allowIn:
+        - robo-components/*
+        - RoboFile.php
+    -
+      function: 'print_r()'
+      message: 'use some logger instead'
+      allowParamsAnywhere:
+        2: true
+    -
+      function: 'in_array()'
+      message: 'set the third parameter $strict to `true` to also check the types to prevent type juggling bugs'
+      allowParamsAnywhere:
+        3: true

robo-components/BootstrapTrait.php

@@ -109,12 +109,12 @@ protected function prepareGithubRepository(string $project_name, string $organiz
 
     $this->taskReplaceInFile('.bootstrap/.ddev/config.yaml')
       ->from('8880')
-      ->to((string) rand(6000, 8000))
+      ->to((string) random_int(6000, 8000))
       ->run();
 
     $this->taskReplaceInFile('.bootstrap/.ddev/config.yaml')
       ->from('4443')
-      ->to((string) rand(3000, 5000))
+      ->to((string) random_int(3000, 5000))
       ->run();
 
     $host_user = $this->taskExec("whoami")

robo-components/DeploymentTrait.php

@@ -609,7 +609,7 @@ public function deployCheckRequirementErrors(string $environment): void {
       if ($requirement['severity'] !== 'Error') {
         continue;
       }
-      if (in_array($requirement['title'], $exclude_list) || in_array($requirement['value'], $exclude_list)) {
+      if (in_array($requirement['title'], $exclude_list, TRUE) || in_array($requirement['value'], $exclude_list, TRUE)) {
         // A warning we decided to exclude.
         continue;
       }
@@ -642,7 +642,7 @@ public function deployPantheonInstallEnv(string $env = 'qa', ?string $pantheon_n
     $forbidden_envs = [
       'live',
     ];
-    if (in_array($env, $forbidden_envs)) {
+    if (in_array($env, $forbidden_envs, TRUE)) {
       throw new \Exception("Reinstalling the site on `$env` environment is forbidden.");
     }
 

web/modules/custom/server_general/server_general.module

@@ -124,7 +124,7 @@ function server_general_field_widget_single_element_moderation_state_default_for
   $bundles = $locked_pages_service->getReferencedBundles();
 
   // Node is not locked or options do not include unpublished, return early.
-  if (!in_array($entity->bundle(), $bundles) || !$locked_pages_service->isNodeLocked($entity) || !isset($element['state']['#options']['unpublished'])) {
+  if (!in_array($entity->bundle(), $bundles, TRUE) || !$locked_pages_service->isNodeLocked($entity) || !isset($element['state']['#options']['unpublished'])) {
     return;
   }
 
@@ -141,7 +141,7 @@ function server_general_node_access(NodeInterface $entity, string $op, AccountIn
   // The bundles that can be locked.
   $bundles = $locked_pages_service->getReferencedBundles();
 
-  if (!in_array($entity->bundle(), $bundles)) {
+  if (!in_array($entity->bundle(), $bundles, TRUE)) {
     return AccessResult::neutral();
   }
 
@@ -321,7 +321,7 @@ function server_general_menu_local_tasks_alter(array &$data, string $route_name)
     'entity.node.edit_form',
   ];
 
-  if (!in_array($route_name, $routes)) {
+  if (!in_array($route_name, $routes, TRUE)) {
     return;
   }
 

web/modules/custom/server_general/src/LockedPages.php

@@ -65,7 +65,7 @@ public function getMainSettings(): ?ContentEntityInterface {
    */
   public function isNodeLocked(NodeInterface $node): bool {
     $restricted_nodes = $this->getRestrictedNodes();
-    return in_array($node->id(), $restricted_nodes);
+    return in_array($node->id(), $restricted_nodes, TRUE);
   }
 
   /**
@@ -83,7 +83,7 @@ protected function getRestrictedNodes(): array {
 
     $locked_entities = $main_settings->get('field_locked_pages')->getValue();
 
-    return array_column($locked_entities, 'target_id');
+    return array_map('strval', array_column($locked_entities, 'target_id'));
   }
 
   /**

web/modules/custom/server_general/src/Routing/LockedPagesRouteSubscriber.php

@@ -85,7 +85,7 @@ public function access(AccountInterface $account, NodeInterface $node): AccessRe
       $cache_tags = $main_settings->getCacheTags();
     }
     // If node is locked, we don't allow accessing the delete page at all.
-    if (in_array($node->getType(), $bundles) && $this->lockedPagesService->isNodeLocked($node)) {
+    if (in_array($node->getType(), $bundles, TRUE) && $this->lockedPagesService->isNodeLocked($node)) {
       $result = AccessResult::forbidden()->addCacheableDependency($node);
       if (!empty($cache_tags)) {
         $result->addCacheTags($cache_tags);

web/modules/custom/server_general/src/ThemeTrait/ElementWrapThemeTrait.php

@@ -207,7 +207,7 @@ public function wrapConditionalContainerBottomPadding(array $element, EntityRefe
       'quote',
     ];
 
-    return in_array($paragraph->bundle(), $paragraph_types_with_no_bottom_padding) ? $element : $this->wrapContainerBottomPadding($element);
+    return in_array($paragraph->bundle(), $paragraph_types_with_no_bottom_padding, TRUE) ? $element : $this->wrapContainerBottomPadding($element);
   }
 
   /**

web/modules/custom/server_migrate/src/Plugin/migrate/process/ImgToMedia.php

@@ -188,7 +188,7 @@ protected function doTransformImages(\DOMDocument $dom, MigrateExecutableInterfa
 
       // Skip transforming external files. Some links may include a host
       // to prod URL, we'll count them as internal files.
-      if (isset($url_parts['host']) && !in_array($url_parts['host'], self::DOMAINS)) {
+      if (isset($url_parts['host']) && !in_array($url_parts['host'], self::DOMAINS, TRUE)) {
         // Absolute URL that's not pointing at production.
         $iterator++;
         continue;

web/modules/custom/server_migrate/src/Plugin/migrate/process/MediaEmbedProcessPluginBase.php

@@ -179,7 +179,7 @@ protected function isImageSource(string $path) {
       // No extension, not an image.
       return FALSE;
     }
-    return in_array($extension, static::MEDIA_IMAGE_VALID_EXTENSIONS);
+    return in_array($extension, static::MEDIA_IMAGE_VALID_EXTENSIONS, TRUE);
   }
 
   /**

web/modules/custom/server_rollbar_test/src/Controller/RollbarError.php

@@ -17,13 +17,13 @@ protected function debug(NodeInterface $node, int $count = 0, array $data = [])
     // Increase complexity by adding nested arrays and objects.
     if ($count < 500) {
       $new_data = [
-        'int' => rand(1, PHP_INT_MAX),
+        'int' => random_int(1, PHP_INT_MAX),
         'nested_array' => array_fill(0, 10, str_repeat('x', 1000)),
         'node_label' => $node->label(),
         'object' => (object) [
-          'id' => uniqid(),
+          'id' => bin2hex(random_bytes(16)),
           'timestamp' => time(),
-          'random' => rand(1, PHP_INT_MAX),
+          'random' => random_int(1, PHP_INT_MAX),
           'deep_nested' => new \stdClass(),
         ],
       ];

web/modules/custom/server_style_guide/src/Controller/StyleGuideController.php

@@ -837,7 +837,7 @@ protected function getPlaceholderImage(int $width, int $height, string $id = '',
    *   URL with placeholder.
    */
   protected function getPlaceholderPersonImage(int $width_and_height) {
-    $unique_id = substr(str_shuffle(md5(microtime())), 0, 10);
+    $unique_id = bin2hex(random_bytes(5));
     return "https://i.pravatar.cc/{$width_and_height}?u=" . $unique_id;
   }