diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/InstancePropertyAssignmentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/InstancePropertyAssignmentAnalyzer.php index 767121217..462544d55 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/InstancePropertyAssignmentAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/InstancePropertyAssignmentAnalyzer.php @@ -463,9 +463,6 @@ class InstancePropertyAssignmentAnalyzer $data_flow_graph = $statements_analyzer->data_flow_graph; - $var_location = new CodeLocation($statements_analyzer->getSource(), $stmt->var); - $property_location = new CodeLocation($statements_analyzer->getSource(), $stmt); - if ($class_storage->specialize_instance) { $var_id = ExpressionIdentifier::getExtendedVarId( $stmt->var, @@ -487,6 +484,8 @@ class InstancePropertyAssignmentAnalyzer return; } + $var_location = new CodeLocation($statements_analyzer->getSource(), $stmt->var); + $var_node = DataFlowNode::getForAssignment( $var_id, $var_location @@ -494,6 +493,8 @@ class InstancePropertyAssignmentAnalyzer $data_flow_graph->addNode($var_node); + $property_location = new CodeLocation($statements_analyzer->getSource(), $stmt); + $property_node = DataFlowNode::getForAssignment( $var_property_id ?: $var_id . '->$property', $property_location @@ -547,77 +548,108 @@ class InstancePropertyAssignmentAnalyzer $statements_analyzer ); - $localized_property_node = DataFlowNode::getForAssignment( + self::taintUnspecializedProperty( + $statements_analyzer, + $stmt, + $property_id, + $class_storage, + $assignment_value_type, + $context, $var_property_id - ?: $property_id . '-' . $property_location->file_name . ':' . $property_location->raw_file_start, - $property_location ); + } + } - $data_flow_graph->addNode($localized_property_node); + public static function taintUnspecializedProperty( + StatementsAnalyzer $statements_analyzer, + PhpParser\Node\Expr $stmt, + string $property_id, + ClassLikeStorage $class_storage, + Union $assignment_value_type, + Context $context, + ?string $var_property_id + ): void { + $codebase = $statements_analyzer->getCodebase(); - $property_node = new DataFlowNode( - $property_id, - $property_id, - null, - null - ); + $data_flow_graph = $statements_analyzer->data_flow_graph; - $data_flow_graph->addNode($property_node); + if (!$data_flow_graph) { + return; + } - $event = new AddRemoveTaintsEvent($stmt, $context, $statements_analyzer, $codebase); + $property_location = new CodeLocation($statements_analyzer->getSource(), $stmt); - $added_taints = $codebase->config->eventDispatcher->dispatchAddTaints($event); - $removed_taints = $codebase->config->eventDispatcher->dispatchRemoveTaints($event); + $localized_property_node = DataFlowNode::getForAssignment( + $var_property_id ?: $property_id, + $property_location + ); - $data_flow_graph->addPath( - $localized_property_node, - $property_node, - 'property-assignment', - $added_taints, - $removed_taints - ); + $data_flow_graph->addNode($localized_property_node); - if ($assignment_value_type->parent_nodes) { - foreach ($assignment_value_type->parent_nodes as $parent_node) { - $data_flow_graph->addPath( - $parent_node, - $localized_property_node, - '=', - $added_taints, - $removed_taints - ); - } - } + $property_node = new DataFlowNode( + $property_id, + $property_id, + null, + null + ); - $declaring_property_class = $codebase->properties->getDeclaringClassForProperty( - $property_id, - false, - $statements_analyzer - ); + $data_flow_graph->addNode($property_node); - if ($statements_analyzer->data_flow_graph instanceof TaintFlowGraph - && $declaring_property_class - && $declaring_property_class !== $class_storage->name - && $stmt->name instanceof PhpParser\Node\Identifier - ) { - $declaring_property_node = new DataFlowNode( - $declaring_property_class . '::$' . $stmt->name, - $declaring_property_class . '::$' . $stmt->name, - null, - null - ); + $event = new AddRemoveTaintsEvent($stmt, $context, $statements_analyzer, $codebase); - $data_flow_graph->addNode($declaring_property_node); + $added_taints = $codebase->config->eventDispatcher->dispatchAddTaints($event); + $removed_taints = $codebase->config->eventDispatcher->dispatchRemoveTaints($event); + $data_flow_graph->addPath( + $localized_property_node, + $property_node, + 'property-assignment', + $added_taints, + $removed_taints + ); + + if ($assignment_value_type->parent_nodes) { + foreach ($assignment_value_type->parent_nodes as $parent_node) { $data_flow_graph->addPath( - $property_node, - $declaring_property_node, - 'property-assignment', + $parent_node, + $localized_property_node, + '=', $added_taints, $removed_taints ); } } + + $declaring_property_class = $codebase->properties->getDeclaringClassForProperty( + $property_id, + false, + $statements_analyzer + ); + + if ($statements_analyzer->data_flow_graph instanceof TaintFlowGraph + && $declaring_property_class + && $declaring_property_class !== $class_storage->name + && ($stmt instanceof PhpParser\Node\Expr\PropertyFetch + || $stmt instanceof PhpParser\Node\Expr\StaticPropertyFetch) + && $stmt->name instanceof PhpParser\Node\Identifier + ) { + $declaring_property_node = new DataFlowNode( + $declaring_property_class . '::$' . $stmt->name, + $declaring_property_class . '::$' . $stmt->name, + null, + null + ); + + $data_flow_graph->addNode($declaring_property_node); + + $data_flow_graph->addPath( + $property_node, + $declaring_property_node, + 'property-assignment', + $added_taints, + $removed_taints + ); + } } /** diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/StaticPropertyAssignmentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/StaticPropertyAssignmentAnalyzer.php index 11aa4d50a..e43f70e09 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/StaticPropertyAssignmentAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/StaticPropertyAssignmentAnalyzer.php @@ -196,6 +196,16 @@ class StaticPropertyAssignmentAnalyzer $context->vars_in_scope[$var_id] = $assignment_value_type; } + InstancePropertyAssignmentAnalyzer::taintUnspecializedProperty( + $statements_analyzer, + $stmt, + $property_id, + $class_storage, + $assignment_value_type, + $context, + null + ); + $class_property_type = $codebase->properties->getPropertyType( $property_id, true, diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php index 4bc2aca3a..f98f99d5f 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php @@ -773,9 +773,6 @@ class AtomicPropertyFetchAnalyzer $data_flow_graph = $statements_analyzer->data_flow_graph; - $var_location = new CodeLocation($statements_analyzer->getSource(), $stmt->var); - $property_location = new CodeLocation($statements_analyzer->getSource(), $stmt); - $added_taints = []; $removed_taints = []; @@ -811,6 +808,9 @@ class AtomicPropertyFetchAnalyzer return; } + $var_location = new CodeLocation($statements_analyzer->getSource(), $stmt->var); + $property_location = new CodeLocation($statements_analyzer->getSource(), $stmt); + $var_node = DataFlowNode::getForAssignment( $var_id, $var_location @@ -849,51 +849,82 @@ class AtomicPropertyFetchAnalyzer $type->parent_nodes = [$property_node->id => $property_node]; } } else { - $var_property_id = ExpressionIdentifier::getExtendedVarId( + self::processUnspecialTaints( + $statements_analyzer, $stmt, - null, - $statements_analyzer - ); - - $localized_property_node = DataFlowNode::getForAssignment( - $var_property_id - ?: $property_id . '-' . $property_location->file_name . ':' . $property_location->raw_file_start, - $property_location - ); - - $data_flow_graph->addNode($localized_property_node); - - $property_node = new DataFlowNode( + $type, $property_id, - $property_id, - null, - null + $in_assignment, + $added_taints, + $removed_taints ); - - $data_flow_graph->addNode($property_node); - - if ($in_assignment) { - $data_flow_graph->addPath( - $localized_property_node, - $property_node, - 'property-assignment', - $added_taints, - $removed_taints - ); - } else { - $data_flow_graph->addPath( - $property_node, - $localized_property_node, - 'property-fetch', - $added_taints, - $removed_taints - ); - } - - $type->parent_nodes = [$localized_property_node->id => $localized_property_node]; } } + /** + * @param ?array $added_taints + * @param ?array $removed_taints + */ + public static function processUnspecialTaints( + StatementsAnalyzer $statements_analyzer, + PhpParser\Node\Expr $stmt, + Union $type, + string $property_id, + bool $in_assignment, + ?array $added_taints, + ?array $removed_taints + ): void { + if (!$statements_analyzer->data_flow_graph) { + return; + } + + $data_flow_graph = $statements_analyzer->data_flow_graph; + + $var_property_id = ExpressionIdentifier::getExtendedVarId( + $stmt, + null, + $statements_analyzer + ); + + $property_location = new CodeLocation($statements_analyzer->getSource(), $stmt); + + $localized_property_node = DataFlowNode::getForAssignment( + $var_property_id ?: $property_id, + $property_location + ); + + $data_flow_graph->addNode($localized_property_node); + + $property_node = new DataFlowNode( + $property_id, + $property_id, + null, + null + ); + + $data_flow_graph->addNode($property_node); + + if ($in_assignment) { + $data_flow_graph->addPath( + $localized_property_node, + $property_node, + 'property-assignment', + $added_taints, + $removed_taints + ); + } else { + $data_flow_graph->addPath( + $property_node, + $localized_property_node, + 'property-fetch', + $added_taints, + $removed_taints + ); + } + + $type->parent_nodes = [$localized_property_node->id => $localized_property_node]; + } + private static function handleEnumName( StatementsAnalyzer $statements_analyzer, PropertyFetch $stmt, diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/StaticPropertyFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/StaticPropertyFetchAnalyzer.php index e28e3f06f..abb687d96 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/StaticPropertyFetchAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/StaticPropertyFetchAnalyzer.php @@ -231,6 +231,16 @@ class StaticPropertyFetchAnalyzer ); } + AtomicPropertyFetchAnalyzer::processUnspecialTaints( + $statements_analyzer, + $stmt, + $stmt_type, + $property_id, + false, + [], + [] + ); + return true; } @@ -389,6 +399,16 @@ class StaticPropertyFetchAnalyzer $stmt_type->getId() ); } + + AtomicPropertyFetchAnalyzer::processUnspecialTaints( + $statements_analyzer, + $stmt, + $stmt_type, + $property_id, + false, + [], + [] + ); } else { $statements_analyzer->node_data->setType($stmt, Type::getMixed()); } diff --git a/tests/TaintTest.php b/tests/TaintTest.php index 8d772e1b4..470c73ccc 100644 --- a/tests/TaintTest.php +++ b/tests/TaintTest.php @@ -2349,6 +2349,22 @@ class TaintTest extends TestCase }', 'error_message' => 'TaintedHtml', ], + 'checkMemoizedStaticMethodCallTaints' => [ + 'code' => ' 'TaintedHtml', + ], ]; }