Huge performance improvements for taint analysis

Author: danog

UPGRADING.md

@@ -2,6 +2,22 @@
 
 ## Changed
 
+- [BC] Taints are now *internally* represented by a bitmap (an integer), instead of an array of strings. Users can still use the usual string taint identifiers (including custom ones, which will be automatically registered by Psalm), but internally, the type of `Psalm\Type\TaintKind` taint types is now an integer.
+
+- [BC] The maximum number of usable taint *types* (including both native taints and custom taints) is now equal to 32 on 32-bit systems and 64 on 64-bit systems: this should be enough for the vast majority of usecases, if more taint types are needed, consider merging some taint types or using some native taint types.  
+
+- [BC] `Psalm\Plugin\EventHandler\AddTaintsInterface::addTaints` and `Psalm\Plugin\EventHandler\RemoveTaintsInterface::removeTaints` now must return an integer taint instead of an array of strings (see the new [taint documentation](https://psalm.dev/docs/security_analysis/custom_taint_sources/) for more info).  
+
+- [BC] The type of the `$taints` parameter of `Psalm\Codebase::addTaintSource` and  `Psalm\Codebase::addTaintSink` was changed to an integer
+
+- [BC] Type of property `Psalm\Storage\FunctionLikeParameter::$sinks` changed from `array|null` to `int`
+
+- [BC] Type of property `Psalm\Storage\FunctionLikeStorage::$taint_source_types` changed from `array` to `int`
+
+- [BC] Type of property `Psalm\Storage\FunctionLikeStorage::$added_taints` changed from `array` to `int`
+
+- [BC] Type of property `Psalm\Storage\FunctionLikeStorage::$removed_taints` changed from `array` to `int`
+
 - [BC] The `startScanningFiles`, `startAnalyzingFiles`, `startAlteringFiles` of `Psalm\Progress\Progress` and subclasses were removed and replaced with a new `startPhase` method, taking a `Psalm\Progress\Phase` enum case.
 
 - [BC] The `start` method was removed, use `expand`, instead; the progress is reset to 0 when changing the current phase.  

dictionaries/InternalTaintSinkMap.php

@@ -5,59 +5,59 @@
 // This maps internal function names to sink types that we don’t want to end up there
 
 /**
- * @var non-empty-array<string, non-empty-list<list<TaintKind::*>>>
+ * @var non-empty-array<string, non-empty-list<int-mask-of<TaintKind::*>>>
  */
 return [
-'exec' => [['shell']],
-'create_function' => [[], ['eval']],
-'file_get_contents' => [['file']],
-'file_put_contents' => [['file']],
-'fopen' => [['file']],
-'unlink' => [['file']],
-'copy' => [['file'], ['file']],
-'file' => [['file']],
-'link' => [['file'], ['file']],
-'mkdir' => [['file']],
-'move_uploaded_file' => [['file'], ['file']],
-'parse_ini_file' => [['file']],
-'chown' => [['file']],
-'lchown' => [['file']],
-'readfile' => [['file']],
-'rename' => [['file'], ['file']],
-'rmdir' => [['file']],
-'header' => [['header']],
-'symlink' => [['file']],
-'tempnam' => [['file']],
-'igbinary_unserialize' => [['unserialize']],
-'ldap_search' => [[], ['ldap'], ['ldap']],
-'mysqli_query' => [[], ['sql']],
-'mysqli::query' => [['sql']],
-'mysqli_real_query' => [[], ['sql']],
-'mysqli::real_query' => [['sql']],
-'mysqli_multi_query' => [[], ['sql']],
-'mysqli::multi_query' => [['sql']],
-'mysqli_prepare' => [[], ['sql']],
-'mysqli::prepare' => [['sql']],
-'mysqli_stmt::__construct' => [[], ['sql']],
-'mysqli_stmt_prepare' => [[], ['sql']],
-'mysqli_stmt::prepare' => [['sql']],
-'passthru' => [['shell']],
-'pcntl_exec' => [['shell']],
-'pg_exec' => [[], ['sql']],
-'pg_prepare' => [[], [], ['sql']],
-'pg_put_line' => [[], ['sql']],
-'pg_query' => [[], ['sql']],
-'pg_query_params' => [[], ['sql']],
-'pg_send_prepare' => [[], [], ['sql']],
-'pg_send_query' => [[], ['sql']],
-'pg_send_query_params' => [[], ['sql'], []],
-'setcookie' => [['cookie'], ['cookie']],
-'shell_exec' => [['shell']],
-'system' => [['shell']],
-'unserialize' => [['unserialize']],
-'popen' => [['shell']],
-'proc_open' => [['shell']],
-'curl_init' => [['ssrf']],
-'curl_setopt' => [[], [], ['ssrf']],
-'getimagesize' => [['ssrf']],
+'exec' => [TaintKind::INPUT_SHELL],
+'create_function' => [0, TaintKind::INPUT_EVAL],
+'file_get_contents' => [TaintKind::INPUT_FILE],
+'file_put_contents' => [TaintKind::INPUT_FILE],
+'fopen' => [TaintKind::INPUT_FILE],
+'unlink' => [TaintKind::INPUT_FILE],
+'copy' => [TaintKind::INPUT_FILE, TaintKind::INPUT_FILE],
+'file' => [TaintKind::INPUT_FILE],
+'link' => [TaintKind::INPUT_FILE, TaintKind::INPUT_FILE],
+'mkdir' => [TaintKind::INPUT_FILE],
+'move_uploaded_file' => [TaintKind::INPUT_FILE, TaintKind::INPUT_FILE],
+'parse_ini_file' => [TaintKind::INPUT_FILE],
+'chown' => [TaintKind::INPUT_FILE],
+'lchown' => [TaintKind::INPUT_FILE],
+'readfile' => [TaintKind::INPUT_FILE],
+'rename' => [TaintKind::INPUT_FILE, TaintKind::INPUT_FILE],
+'rmdir' => [TaintKind::INPUT_FILE],
+'header' => [TaintKind::INPUT_HEADER],
+'symlink' => [TaintKind::INPUT_FILE],
+'tempnam' => [TaintKind::INPUT_FILE],
+'igbinary_unserialize' => [TaintKind::INPUT_UNSERIALIZE],
+'ldap_search' => [0, TaintKind::INPUT_LDAP, TaintKind::INPUT_LDAP],
+'mysqli_query' => [0, TaintKind::INPUT_SQL],
+'mysqli::query' => [TaintKind::INPUT_SQL],
+'mysqli_real_query' => [0, TaintKind::INPUT_SQL],
+'mysqli::real_query' => [TaintKind::INPUT_SQL],
+'mysqli_multi_query' => [0, TaintKind::INPUT_SQL],
+'mysqli::multi_query' => [TaintKind::INPUT_SQL],
+'mysqli_prepare' => [0, TaintKind::INPUT_SQL],
+'mysqli::prepare' => [TaintKind::INPUT_SQL],
+'mysqli_stmt::__construct' => [0, TaintKind::INPUT_SQL],
+'mysqli_stmt_prepare' => [0, TaintKind::INPUT_SQL],
+'mysqli_stmt::prepare' => [TaintKind::INPUT_SQL],
+'passthru' => [TaintKind::INPUT_SHELL],
+'pcntl_exec' => [TaintKind::INPUT_SHELL],
+'pg_exec' => [0, TaintKind::INPUT_SQL],
+'pg_prepare' => [0, 0, TaintKind::INPUT_SQL],
+'pg_put_line' => [0, TaintKind::INPUT_SQL],
+'pg_query' => [0, TaintKind::INPUT_SQL],
+'pg_query_params' => [0, TaintKind::INPUT_SQL],
+'pg_send_prepare' => [0, 0, TaintKind::INPUT_SQL],
+'pg_send_query' => [0, TaintKind::INPUT_SQL],
+'pg_send_query_params' => [0, TaintKind::INPUT_SQL, 0],
+'setcookie' => [TaintKind::INPUT_COOKIE, TaintKind::INPUT_COOKIE],
+'shell_exec' => [TaintKind::INPUT_SHELL],
+'system' => [TaintKind::INPUT_SHELL],
+'unserialize' => [TaintKind::INPUT_UNSERIALIZE],
+'popen' => [TaintKind::INPUT_SHELL],
+'proc_open' => [TaintKind::INPUT_SHELL],
+'curl_init' => [TaintKind::INPUT_SSRF],
+'curl_setopt' => [0, 0, TaintKind::INPUT_SSRF],
+'getimagesize' => [TaintKind::INPUT_SSRF],
 ];

docs/security_analysis/custom_taint_sources.md

@@ -26,29 +26,70 @@ For example this plugin treats all variables named `$bad_data` as taint sources.
 namespace Psalm\Example\Plugin;
 
 use PhpParser\Node\Expr\Variable;
+use Psalm\Codebase;
 use Psalm\Plugin\EventHandler\AddTaintsInterface;
 use Psalm\Plugin\EventHandler\Event\AddRemoveTaintsEvent;
-use Psalm\Type\TaintKindGroup;
+use Psalm\Type\TaintKind;
 
 /**
- * Add input taints to all variables named 'bad_data'
+ * Add input taints to all variables named 'bad_data' or 'even_badder_data'.
+ * 
+ * RemoveTaintsInterface is also available to remove taints.
  */
 class TaintBadDataPlugin implements AddTaintsInterface
 {
+    private static int $myCustomTaint;
+    private static int $myCustomTaintAlias;
+    /**
+     * Must be called by the PluginEntryPointInterface (__invoke) of your plugin.
+     */
+    public static function init(Codebase $codebase): void
+    {
+        // Register a new custom taint
+        // The taint name may be used in @psalm-taint-* annotations in the code.
+        self::$myCustomTaint = $codebase->getOrRegisterTaint("my_custom_taint");
+
+        // Register a taint alias that combines multiple pre-registered taint types
+        // Taint alias names may be used in @psalm-taint-* annotations in the code.
+        self::$myCustomTaintAlias = $codebase->registerTaintAlias(
+            "my_custom_taint_alias",
+            self::$myCustomTaint | TaintKind::ALL_INPUT
+        );
+    }
+
     /**
      * Called to see what taints should be added
      *
-     * @return list<string>
+     * @return int A bitmap of taint from the IDs
      */
-    public static function addTaints(AddRemoveTaintsEvent $event): array
+    public static function addTaints(AddRemoveTaintsEvent $event): int
     {
         $expr = $event->getExpr();
 
         if ($expr instanceof Variable && $expr->name === 'bad_data') {
-            return TaintKindGroup::ALL_INPUT;
+            return TaintKind::ALL_INPUT;
+        }
+
+        if ($expr instanceof Variable && $expr->name === 'even_badder_data') {
+            return self::$myCustomTaint;
+        }
+
+        if ($expr instanceof Variable && $expr->name === 'even_badder_data_2') {
+            return self::$myCustomTaintAlias;
+        }
+
+        if ($expr instanceof Variable && $expr->name === 'secret_even_badder_data_3') {
+            // Combine taints using |
+            return self::$myCustomTaintAlias | USER_SECRET;
+        }
+
+        if ($expr instanceof Variable && $expr->name === 'bad_data_but_ok_cookie') {
+            // Remove taints using & and ~ to negate a taint (group)
+            return self::$myCustomTaintAlias & ~TaintKind::INPUT_COOKIE;
         }
 
-        return [];
+        // No taints
+        return 0;
     }
 }
 ```

examples/TemplateChecker.php

@@ -178,6 +178,7 @@ private function checkWithViewClass(Context $context, array $stmts): void
         $statements_source = new StatementsAnalyzer(
             $view_method_analyzer,
             new NodeDataProvider(),
+            false,
         );
 
         $statements_source->analyze($pseudo_method_stmts, $context);

examples/plugins/SafeArrayKeyChecker.php

@@ -6,21 +6,22 @@
 use Psalm\Internal\Analyzer\StatementsAnalyzer;
 use Psalm\Plugin\EventHandler\Event\AddRemoveTaintsEvent;
 use Psalm\Plugin\EventHandler\RemoveTaintsInterface;
+use Psalm\Type\TaintKind;
 
 final class SafeArrayKeyChecker implements RemoveTaintsInterface
 {
     /**
      * Called to see what taints should be removed
      *
-     * @return list<string>
+     * @return int
      */
     #[\Override]
-    public static function removeTaints(AddRemoveTaintsEvent $event): array
+    public static function removeTaints(AddRemoveTaintsEvent $event): int
     {
         $item = $event->getExpr();
         $statements_analyzer = $event->getStatementsSource();
         if (!($item instanceof ArrayItem) || !($statements_analyzer instanceof StatementsAnalyzer)) {
-            return [];
+            return 0;
         }
         $item_key_value = '';
         if ($item->key) {
@@ -34,8 +35,8 @@ public static function removeTaints(AddRemoveTaintsEvent $event): array
         }
 
         if ($item_key_value === 'safe_key') {
-            return ['html'];
+            return TaintKind::INPUT_HTML;
         }
-        return [];
+        return 0;
     }
 }

examples/plugins/TaintActiveRecords.php

@@ -12,6 +12,7 @@
 use Psalm\Plugin\EventHandler\Event\AddRemoveTaintsEvent;
 use Psalm\Type\Atomic;
 use Psalm\Type\Atomic\TNamedObject;
+use Psalm\Type\TaintKind;
 use Psalm\Type\TaintKindGroup;
 use Psalm\Type\Union;
 
@@ -27,16 +28,16 @@ final class TaintActiveRecords implements AddTaintsInterface
     /**
      * Called to see what taints should be added
      *
-     * @return list<string>
+     * @return int
      */
     #[Override]
-    public static function addTaints(AddRemoveTaintsEvent $event): array
+    public static function addTaints(AddRemoveTaintsEvent $event): int
     {
         $expr = $event->getExpr();
 
         // Model properties are accessed by property fetch, so abort here
         if ($expr instanceof ArrayItem) {
-            return [];
+            return 0;
         }
 
         $statements_source = $event->getStatementsSource();
@@ -51,11 +52,11 @@ public static function addTaints(AddRemoveTaintsEvent $event): array
             }
 
             if (self::containsActiveRecord($expr_type)) {
-                return TaintKindGroup::ALL_INPUT;
+                return TaintKind::ALL_INPUT;
             }
         } while ($expr = self::getParentNode($expr));
 
-        return [];
+        return 0;
     }
 
     /**

psalm-baseline.xml

@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<files psalm-version="6.x-dev@4258dc813b28c2b03865f34a17ddf072f006b357">
+<files psalm-version="dev-master@b513b32ef4a8f5a46c4412133f345182e34db54c">
   <file src="examples/TemplateChecker.php">
     <PossiblyUndefinedIntArrayOffset>
       <code><![CDATA[$comment_block->tags['variablesfrom'][0]]]></code>
@@ -911,7 +911,6 @@
       <code><![CDATA[$context->calling_method_id]]></code>
       <code><![CDATA[$context->calling_method_id]]></code>
       <code><![CDATA[$context->calling_method_id]]></code>
-      <code><![CDATA[$function_param->sinks]]></code>
       <code><![CDATA[$function_params]]></code>
       <code><![CDATA[$function_params]]></code>
       <code><![CDATA[$function_params]]></code>
@@ -1041,7 +1040,6 @@
       <code><![CDATA[$method_id]]></code>
     </ImplicitToStringCast>
     <RiskyTruthyFalsyComparison>
-      <code><![CDATA[!$parent_node->specialization_key]]></code>
       <code><![CDATA[$var_id]]></code>
     </RiskyTruthyFalsyComparison>
   </file>
@@ -1621,12 +1619,7 @@
   </file>
   <file src="src/Psalm/Internal/Codebase/TaintFlowGraph.php">
     <RiskyTruthyFalsyComparison>
-      <code><![CDATA[$node->specialization_key]]></code>
-      <code><![CDATA[$node->unspecialized_id]]></code>
-      <code><![CDATA[$path->escaped_taints]]></code>
-      <code><![CDATA[$path->unescaped_taints]]></code>
-      <code><![CDATA[$source->specialization_key]]></code>
-      <code><![CDATA[end($source->path_types)]]></code>
+      <code><![CDATA[end($path_types)]]></code>
     </RiskyTruthyFalsyComparison>
   </file>
   <file src="src/Psalm/Internal/Composer.php">
@@ -1635,9 +1628,14 @@
     </RiskyTruthyFalsyComparison>
   </file>
   <file src="src/Psalm/Internal/DataFlow/DataFlowNode.php">
-    <RiskyTruthyFalsyComparison>
-      <code><![CDATA[$specialization_key]]></code>
-    </RiskyTruthyFalsyComparison>
+    <RedundantPropertyInitializationCheck>
+      <code><![CDATA[new self('closure-use', null, null, 'closure use')]]></code>
+      <code><![CDATA[new self('unknown-origin', null, null, 'unknown origin')]]></code>
+      <code><![CDATA[new self('variable-use', null, null, 'variable use')]]></code>
+      <code><![CDATA[self::$forClosureUse]]></code>
+      <code><![CDATA[self::$forUnknownOrigin]]></code>
+      <code><![CDATA[self::$forVariableUse]]></code>
+    </RedundantPropertyInitializationCheck>
   </file>
   <file src="src/Psalm/Internal/Diff/ClassStatementsDiffer.php">
     <ImplicitToStringCast>