From b7910cae240c9887076999f7edd78f200d85ccf5 Mon Sep 17 00:00:00 2001 From: Patrick Remy Date: Tue, 19 Sep 2023 20:32:26 +0200 Subject: [PATCH] refactor: only add taint sources if required --- .../Statements/Expression/ArrayAnalyzer.php | 22 +++++++--------- .../InstancePropertyAssignmentAnalyzer.php | 26 +++++++------------ .../Expression/AssignmentAnalyzer.php | 11 ++++---- .../Expression/BinaryOpAnalyzer.php | 3 +-- .../Expression/Call/ArgumentAnalyzer.php | 3 +-- .../Expression/Call/FunctionCallAnalyzer.php | 7 ++--- .../Expression/Call/NewAnalyzer.php | 9 ++++--- .../Expression/EncapsulatedStringAnalyzer.php | 11 ++++---- .../Statements/Expression/EvalAnalyzer.php | 9 ++++--- .../Expression/Fetch/ArrayFetchAnalyzer.php | 3 +-- .../Fetch/AtomicPropertyFetchAnalyzer.php | 6 ++--- .../Statements/Expression/IncludeAnalyzer.php | 9 ++++--- 12 files changed, 54 insertions(+), 65 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php index 2d252f53d51..a235f9dfbd3 100644 --- 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/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/InstancePropertyAssignmentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/InstancePropertyAssignmentAnalyzer.php index 0a67a84ea9a..1928ccffef3 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/InstancePropertyAssignmentAnalyzer.php +++ b/src/Psalm/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 index 7344eecea94..2feee5d98f6 100644 --- 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 index 4070f927e0c..cc9171aacec 100644 --- 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 index 9c56b6c1c04..cefa80c8ede 100644 --- 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 index 870b46d7ba0..1dfeb4764a0 100644 --- 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 index a9645432ebc..1cfc95946c7 100644 --- 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 index b1851fe7a92..a3c5fa9eb26 100644 --- 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 index f6c348c9bd1..123fe2f8d0d 100644 --- 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 index 7d78532696e..e6301aa5dc4 100644 --- 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 index 2f1e328f555..f25f5d7626f 100644 --- 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 index 0ad7f4151a8..46ef17d33c7 100644 --- 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) {