refactor: only add taint sources if required

vimeo · Sep 19, 2023 · b7910ca · b7910ca
1 parent 2fd69f5
commit b7910ca
Show file tree

Hide file tree

Showing 12 changed files with 54 additions and 65 deletions.
diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php
@@ -411,6 +411,11 @@ private static function analyzeArrayItem(
                     $added_taints = $codebase->config->eventDispatcher->dispatchAddTaints($event);
                     $removed_taints = $codebase->config->eventDispatcher->dispatchRemoveTaints($event);
 
+                    if ($added_taints !== [] && $statements_analyzer->data_flow_graph instanceof TaintFlowGraph) {
+                        $taint_source = TaintSource::fromNode($new_parent_node);
+                        $statements_analyzer->data_flow_graph->addSource($taint_source);
+                    }
+
                     foreach ($item_value_type->parent_nodes as $parent_node) {
                         $data_flow_graph->addPath(
                             $parent_node,
@@ -423,12 +428,6 @@ private static function analyzeArrayItem(
                     }
 
                     $array_creation_info->parent_taint_nodes += [$new_parent_node->id => $new_parent_node];
-
-                    if ($statements_analyzer->data_flow_graph instanceof TaintFlowGraph) {
-                        $taint_source = TaintSource::fromNode($new_parent_node);
-                        $statements_analyzer->data_flow_graph->addSource($taint_source);
-                        $item_value_type = $item_value_type->addParentNodes([$taint_source->id => $taint_source]);
-                    }
                 }
 
                 if ($item_key_type
@@ -452,6 +451,11 @@ private static function analyzeArrayItem(
                     $added_taints = $codebase->config->eventDispatcher->dispatchAddTaints($event);
                     $removed_taints = $codebase->config->eventDispatcher->dispatchRemoveTaints($event);
 
+                    if ($added_taints !== [] && $statements_analyzer->data_flow_graph instanceof TaintFlowGraph) {
+                        $taint_source = TaintSource::fromNode($new_parent_node);
+                        $statements_analyzer->data_flow_graph->addSource($taint_source);
+                    }
+
                     foreach ($item_key_type->parent_nodes as $parent_node) {
                         $data_flow_graph->addPath(
                             $parent_node,
@@ -463,12 +467,6 @@ private static function analyzeArrayItem(
                     }
 
                     $array_creation_info->parent_taint_nodes += [$new_parent_node->id => $new_parent_node];
-
-                    if ($statements_analyzer->data_flow_graph instanceof TaintFlowGraph) {
-                        $taint_source = TaintSource::fromNode($new_parent_node);
-                        $statements_analyzer->data_flow_graph->addSource($taint_source);
-                        $item_value_type = $item_value_type->addParentNodes([$taint_source->id => $taint_source]);
-                    }
                 }
             }
         }

diff --git a/...Internal/Analyzer/Statements/Expression/Assignment/InstancePropertyAssignmentAnalyzer.php b/...Internal/Analyzer/Statements/Expression/Assignment/InstancePropertyAssignmentAnalyzer.php
@@ -504,6 +504,11 @@ private static function taintProperty(
                 $added_taints = $codebase->config->eventDispatcher->dispatchAddTaints($event);
                 $removed_taints = $codebase->config->eventDispatcher->dispatchRemoveTaints($event);
 
+                if ($added_taints !== [] && $statements_analyzer->data_flow_graph instanceof TaintFlowGraph) {
+                    $taint_source = TaintSource::fromNode($property_node);
+                    $statements_analyzer->data_flow_graph->addSource($taint_source);
+                }
+
                 $data_flow_graph->addPath(
                     $property_node,
                     $var_node,
@@ -519,14 +524,6 @@ private static function taintProperty(
                     }
                 }
 
-                if ($statements_analyzer->data_flow_graph instanceof TaintFlowGraph) {
-                    $taint_source = TaintSource::fromNode($property_node);
-                    $statements_analyzer->data_flow_graph->addSource($taint_source);
-                    $assignment_value_type = $assignment_value_type->addParentNodes([
-                        $taint_source->id => $taint_source,
-                    ]);
-                }
-
                 if (isset($context->vars_in_scope[$var_id])) {
                     $stmt_var_type = $context->vars_in_scope[$var_id]->setParentNodes(
                         [$var_node->id => $var_node],
@@ -607,6 +604,11 @@ public static function taintUnspecializedProperty(
         $added_taints = $codebase->config->eventDispatcher->dispatchAddTaints($event);
         $removed_taints = $codebase->config->eventDispatcher->dispatchRemoveTaints($event);
 
+        if ($added_taints !== [] && $statements_analyzer->data_flow_graph instanceof TaintFlowGraph) {
+            $taint_source = TaintSource::fromNode($property_node);
+            $statements_analyzer->data_flow_graph->addSource($taint_source);
+        }
+
         $data_flow_graph->addPath(
             $localized_property_node,
             $property_node,
@@ -627,14 +629,6 @@ public static function taintUnspecializedProperty(
             }
         }
 
-        if ($statements_analyzer->data_flow_graph instanceof TaintFlowGraph) {
-            $taint_source = TaintSource::fromNode($property_node);
-            $statements_analyzer->data_flow_graph->addSource($taint_source);
-            $assignment_value_type = $assignment_value_type->addParentNodes([
-                $taint_source->id => $taint_source,
-            ]);
-        }
-
         $declaring_property_class = $codebase->properties->getDeclaringClassForProperty(
             $property_id,
             false,

diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php
@@ -822,6 +822,11 @@ private static function taintAssignment(
         $data_flow_graph->addNode($new_parent_node);
         $new_parent_nodes = [$new_parent_node->id => $new_parent_node];
 
+        if ($added_taints !== [] && $data_flow_graph instanceof TaintFlowGraph) {
+            $taint_source = TaintSource::fromNode($new_parent_node);
+            $data_flow_graph->addSource($taint_source);
+        }
+
         foreach ($parent_nodes as $parent_node) {
             $data_flow_graph->addPath(
                 $parent_node,
@@ -833,12 +838,6 @@ private static function taintAssignment(
         }
 
         $type = $type->setParentNodes($new_parent_nodes, false);
-
-        if ($data_flow_graph instanceof TaintFlowGraph) {
-            $taint_source = TaintSource::fromNode($new_parent_node);
-            $data_flow_graph->addSource($taint_source);
-            $type = $type->addParentNodes([$taint_source->id => $taint_source]);
-        }
     }
 
     public static function analyzeAssignmentOperation(

diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php
@@ -170,10 +170,9 @@ public static function analyze(
                 $added_taints = $codebase->config->eventDispatcher->dispatchAddTaints($event);
                 $removed_taints = $codebase->config->eventDispatcher->dispatchRemoveTaints($event);
 
-                if ($statements_analyzer->data_flow_graph instanceof TaintFlowGraph) {
+                if ($added_taints !== [] && $statements_analyzer->data_flow_graph instanceof TaintFlowGraph) {
                     $taint_source = TaintSource::fromNode($new_parent_node);
                     $statements_analyzer->data_flow_graph->addSource($taint_source);
-                    $stmt_type = $stmt_type->addParentNodes([$taint_source->id => $taint_source]);
                 }
 
                 if ($stmt_left_type && $stmt_left_type->parent_nodes) {

diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php
@@ -1619,10 +1619,9 @@ private static function processTaintedness(
             );
         }
 
-        if ($statements_analyzer->data_flow_graph instanceof TaintFlowGraph) {
+        if ($added_taints !== [] && $statements_analyzer->data_flow_graph instanceof TaintFlowGraph) {
             $taint_source = TaintSource::fromNode($argument_value_node);
             $statements_analyzer->data_flow_graph->addSource($taint_source);
-            $input_type = $input_type->addParentNodes([$taint_source->id => $taint_source]);
         }
     }
 }
diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php
@@ -873,9 +873,10 @@ private static function getAnalyzeNamedExpression(
                     );
                 }
 
-                $taint_source = TaintSource::fromNode($custom_call_sink);
-                $statements_analyzer->data_flow_graph->addSource($taint_source);
-                $stmt_name_type = $stmt_name_type->addParentNodes([$taint_source->id => $taint_source]);
+                if ($added_taints !== []) {
+                    $taint_source = TaintSource::fromNode($custom_call_sink);
+                    $statements_analyzer->data_flow_graph->addSource($taint_source);
+                }
             }
         }
 

diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php
@@ -678,6 +678,11 @@ private static function analyzeConstructorExpression(
                 $added_taints = $codebase->config->eventDispatcher->dispatchAddTaints($event);
                 $removed_taints = $codebase->config->eventDispatcher->dispatchRemoveTaints($event);
 
+                if ($added_taints !== []) {
+                    $taint_source = TaintSource::fromNode($custom_call_sink);
+                    $statements_analyzer->data_flow_graph->addSource($taint_source);
+                }
+
                 foreach ($stmt_class_type->parent_nodes as $parent_node) {
                     $statements_analyzer->data_flow_graph->addPath(
                         $parent_node,
@@ -687,10 +692,6 @@ private static function analyzeConstructorExpression(
                         $removed_taints,
                     );
                 }
-
-                $taint_source = TaintSource::fromNode($custom_call_sink);
-                $statements_analyzer->data_flow_graph->addSource($taint_source);
-                $stmt_class_type = $stmt_class_type->addParentNodes([$taint_source->id => $taint_source]);
             }
 
             if (self::checkMethodArgs(

diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/EncapsulatedStringAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/EncapsulatedStringAnalyzer.php
@@ -100,6 +100,11 @@ public static function analyze(
                     $added_taints = $codebase->config->eventDispatcher->dispatchAddTaints($event);
                     $removed_taints = $codebase->config->eventDispatcher->dispatchRemoveTaints($event);
 
+                    if ($added_taints !== [] && $statements_analyzer->data_flow_graph instanceof TaintFlowGraph) {
+                        $taint_source = TaintSource::fromNode($new_parent_node);
+                        $statements_analyzer->data_flow_graph->addSource($taint_source);
+                    }
+
                     if ($casted_part_type->parent_nodes) {
                         foreach ($casted_part_type->parent_nodes as $parent_node) {
                             $statements_analyzer->data_flow_graph->addPath(
@@ -111,12 +116,6 @@ public static function analyze(
                             );
                         }
                     }
-
-                    if ($statements_analyzer->data_flow_graph instanceof TaintFlowGraph) {
-                        $taint_source = TaintSource::fromNode($new_parent_node);
-                        $statements_analyzer->data_flow_graph->addSource($taint_source);
-                        $casted_part_type = $casted_part_type->addParentNodes([$taint_source->id => $taint_source]);
-                    }
                 }
             } elseif ($part instanceof EncapsedStringPart) {
                 if ($literal_string !== null) {

diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/EvalAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/EvalAnalyzer.php
@@ -61,6 +61,11 @@ public static function analyze(
                 $added_taints = $codebase->config->eventDispatcher->dispatchAddTaints($event);
                 $removed_taints = $codebase->config->eventDispatcher->dispatchRemoveTaints($event);
 
+                if ($added_taints !== []) {
+                    $taint_source = TaintSource::fromNode($eval_param_sink);
+                    $statements_analyzer->data_flow_graph->addSource($taint_source);
+                }
+
                 foreach ($expr_type->parent_nodes as $parent_node) {
                     $statements_analyzer->data_flow_graph->addPath(
                         $parent_node,
@@ -70,10 +75,6 @@ public static function analyze(
                         $removed_taints,
                     );
                 }
-
-                $taint_source = TaintSource::fromNode($eval_param_sink);
-                $statements_analyzer->data_flow_graph->addSource($taint_source);
-                $expr_type = $expr_type->addParentNodes([$taint_source->id => $taint_source]);
             }
         }
 

diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php
@@ -448,10 +448,9 @@ public static function taintArrayFetch(
 
             $stmt_type = $stmt_type->setParentNodes([$new_parent_node->id => $new_parent_node]);
 
-            if ($statements_analyzer->data_flow_graph instanceof TaintFlowGraph) {
+            if ($added_taints !== [] && $statements_analyzer->data_flow_graph instanceof TaintFlowGraph) {
                 $taint_source = TaintSource::fromNode($new_parent_node);
                 $statements_analyzer->data_flow_graph->addSource($taint_source);
-                $stmt_type = $stmt_type->addParentNodes([$taint_source->id => $taint_source]);
             }
 
             if ($array_key_node) {

diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php
@@ -900,10 +900,9 @@ public static function processTaints(
 
                 $type = $type->setParentNodes([$property_node->id => $property_node], true);
 
-                if ($statements_analyzer->data_flow_graph instanceof TaintFlowGraph) {
+                if ($added_taints !== [] && $statements_analyzer->data_flow_graph instanceof TaintFlowGraph) {
                     $taint_source = TaintSource::fromNode($var_node);
                     $statements_analyzer->data_flow_graph->addSource($taint_source);
-                    $type = $type->addParentNodes([$taint_source->id => $taint_source]);
                 }
             }
         } else {
@@ -982,10 +981,9 @@ public static function processUnspecialTaints(
 
         $type = $type->setParentNodes([$localized_property_node->id => $localized_property_node], true);
 
-        if ($statements_analyzer->data_flow_graph instanceof TaintFlowGraph) {
+        if ($added_taints !== [] && $statements_analyzer->data_flow_graph instanceof TaintFlowGraph) {
             $taint_source = TaintSource::fromNode($localized_property_node);
             $statements_analyzer->data_flow_graph->addSource($taint_source);
-            $type = $type->addParentNodes([$taint_source->id => $taint_source]);
         }
     }
 

diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/IncludeAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/IncludeAnalyzer.php
@@ -138,6 +138,11 @@ public static function analyze(
             $added_taints = $codebase->config->eventDispatcher->dispatchAddTaints($event);
             $removed_taints = $codebase->config->eventDispatcher->dispatchRemoveTaints($event);
 
+            if (!$added_taints !== []) {
+                $taint_source = TaintSource::fromNode($include_param_sink);
+                $statements_analyzer->data_flow_graph->addSource($taint_source);
+            }
+
             foreach ($stmt_expr_type->parent_nodes as $parent_node) {
                 $statements_analyzer->data_flow_graph->addPath(
                     $parent_node,
@@ -147,10 +152,6 @@ public static function analyze(
                     $removed_taints,
                 );
             }
-
-            $taint_source = TaintSource::fromNode($include_param_sink);
-            $statements_analyzer->data_flow_graph->addSource($taint_source);
-            $stmt_expr_type = $stmt_expr_type->addParentNodes([$taint_source->id => $taint_source]);
         }
 
         if ($path_to_file) {