From 63ddceaf8ed81785df33d78ad0ba9d12caeb4579 Mon Sep 17 00:00:00 2001 From: Adrien LUCAS Date: Wed, 6 Jan 2021 20:14:52 +0100 Subject: [PATCH] Taint specialized calls even when not using a variable (#4940) --- .../Method/MethodCallReturnTypeFetcher.php | 214 +++++++++--------- tests/TaintTest.php | 26 +++ 2 files changed, 132 insertions(+), 108 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/MethodCallReturnTypeFetcher.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/MethodCallReturnTypeFetcher.php index d235850b3..7fc8e8738 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/MethodCallReturnTypeFetcher.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/MethodCallReturnTypeFetcher.php @@ -241,130 +241,128 @@ class MethodCallReturnTypeFetcher $is_declaring = (string) $declaring_method_id === (string) $method_id; - if ($method_storage->specialize_call) { - $var_id = ExpressionIdentifier::getArrayVarId( - $var_expr, - null, - $statements_analyzer + $var_id = ExpressionIdentifier::getArrayVarId( + $var_expr, + null, + $statements_analyzer + ); + + if ($method_storage->specialize_call && $var_id && isset($context->vars_in_scope[$var_id])) { + $var_nodes = []; + + $parent_nodes = $context->vars_in_scope[$var_id]->parent_nodes; + + $unspecialized_parent_nodes = \array_filter( + $parent_nodes, + function ($parent_node) { + return !$parent_node->specialization_key; + } ); - if ($var_id && isset($context->vars_in_scope[$var_id])) { - $var_nodes = []; - - $parent_nodes = $context->vars_in_scope[$var_id]->parent_nodes; - - $unspecialized_parent_nodes = \array_filter( - $parent_nodes, - function ($parent_node) { - return !$parent_node->specialization_key; - } - ); - - $specialized_parent_nodes = \array_filter( - $parent_nodes, - function ($parent_node) { - return (bool) $parent_node->specialization_key; - } - ); - - $var_node = DataFlowNode::getForAssignment( - $var_id, - new CodeLocation($statements_analyzer, $var_expr) - ); - - if ($method_storage->location) { - $this_parent_node = DataFlowNode::getForAssignment( - '$this in ' . $method_id, - $method_storage->location - ); - - foreach ($parent_nodes as $parent_node) { - $statements_analyzer->data_flow_graph->addPath($parent_node, $this_parent_node, '='); - } + $specialized_parent_nodes = \array_filter( + $parent_nodes, + function ($parent_node) { + return (bool) $parent_node->specialization_key; } + ); - $var_nodes[$var_node->id] = $var_node; + $var_node = DataFlowNode::getForAssignment( + $var_id, + new CodeLocation($statements_analyzer, $var_expr) + ); - $method_call_nodes = []; + if ($method_storage->location) { + $this_parent_node = DataFlowNode::getForAssignment( + '$this in ' . $method_id, + $method_storage->location + ); - if ($unspecialized_parent_nodes) { - $method_call_node = DataFlowNode::getForMethodReturn( - (string) $method_id, - $cased_method_id, - $is_declaring ? ($method_storage->signature_return_type_location - ?: $method_storage->location) : null, - $node_location - ); - - $method_call_nodes[$method_call_node->id] = $method_call_node; + foreach ($parent_nodes as $parent_node) { + $statements_analyzer->data_flow_graph->addPath($parent_node, $this_parent_node, '='); } + } - foreach ($specialized_parent_nodes as $parent_node) { - $universal_method_call_node = DataFlowNode::getForMethodReturn( - (string) $method_id, - $cased_method_id, - $is_declaring ? ($method_storage->signature_return_type_location - ?: $method_storage->location) : null, - null - ); + $var_nodes[$var_node->id] = $var_node; - $method_call_node = new DataFlowNode( - strtolower((string) $method_id), - $cased_method_id, - $is_declaring ? ($method_storage->signature_return_type_location - ?: $method_storage->location) : null, - $parent_node->specialization_key - ); + $method_call_nodes = []; + + if ($unspecialized_parent_nodes) { + $method_call_node = DataFlowNode::getForMethodReturn( + (string) $method_id, + $cased_method_id, + $is_declaring ? ($method_storage->signature_return_type_location + ?: $method_storage->location) : null, + $node_location + ); + + $method_call_nodes[$method_call_node->id] = $method_call_node; + } + + foreach ($specialized_parent_nodes as $parent_node) { + $universal_method_call_node = DataFlowNode::getForMethodReturn( + (string) $method_id, + $cased_method_id, + $is_declaring ? ($method_storage->signature_return_type_location + ?: $method_storage->location) : null, + null + ); + + $method_call_node = new DataFlowNode( + strtolower((string) $method_id), + $cased_method_id, + $is_declaring ? ($method_storage->signature_return_type_location + ?: $method_storage->location) : null, + $parent_node->specialization_key + ); + + $statements_analyzer->data_flow_graph->addPath( + $universal_method_call_node, + $method_call_node, + '=' + ); + + $method_call_nodes[$method_call_node->id] = $method_call_node; + } + + foreach ($method_call_nodes as $method_call_node) { + $statements_analyzer->data_flow_graph->addNode($method_call_node); + + foreach ($var_nodes as $var_node) { + $statements_analyzer->data_flow_graph->addNode($var_node); $statements_analyzer->data_flow_graph->addPath( - $universal_method_call_node, $method_call_node, - '=' + $var_node, + 'method-call-' . $method_id->method_name + ); + } + + if (!$is_declaring) { + $cased_declaring_method_id = $codebase->methods->getCasedMethodId($declaring_method_id); + + $declaring_method_call_node = new DataFlowNode( + strtolower((string) $declaring_method_id), + $cased_declaring_method_id, + $method_storage->signature_return_type_location ?: $method_storage->location, + $method_call_node->specialization_key ); - $method_call_nodes[$method_call_node->id] = $method_call_node; + $statements_analyzer->data_flow_graph->addNode($declaring_method_call_node); + $statements_analyzer->data_flow_graph->addPath( + $declaring_method_call_node, + $method_call_node, + 'parent' + ); } - - foreach ($method_call_nodes as $method_call_node) { - $statements_analyzer->data_flow_graph->addNode($method_call_node); - - foreach ($var_nodes as $var_node) { - $statements_analyzer->data_flow_graph->addNode($var_node); - - $statements_analyzer->data_flow_graph->addPath( - $method_call_node, - $var_node, - 'method-call-' . $method_id->method_name - ); - } - - if (!$is_declaring) { - $cased_declaring_method_id = $codebase->methods->getCasedMethodId($declaring_method_id); - - $declaring_method_call_node = new DataFlowNode( - strtolower((string) $declaring_method_id), - $cased_declaring_method_id, - $method_storage->signature_return_type_location ?: $method_storage->location, - $method_call_node->specialization_key - ); - - $statements_analyzer->data_flow_graph->addNode($declaring_method_call_node); - $statements_analyzer->data_flow_graph->addPath( - $declaring_method_call_node, - $method_call_node, - 'parent' - ); - } - } - - $return_type_candidate->parent_nodes = $method_call_nodes; - - $stmt_var_type = clone $context->vars_in_scope[$var_id]; - - $stmt_var_type->parent_nodes = $var_nodes; - - $context->vars_in_scope[$var_id] = $stmt_var_type; } + + $return_type_candidate->parent_nodes = $method_call_nodes; + + $stmt_var_type = clone $context->vars_in_scope[$var_id]; + + $stmt_var_type->parent_nodes = $var_nodes; + + $context->vars_in_scope[$var_id] = $stmt_var_type; } else { $method_call_node = DataFlowNode::getForMethodReturn( (string) $method_id, diff --git a/tests/TaintTest.php b/tests/TaintTest.php index fb8fe9d04..05a08683a 100644 --- a/tests/TaintTest.php +++ b/tests/TaintTest.php @@ -1591,6 +1591,32 @@ class TaintTest extends TestCase echo $a->isUnsafe();', 'error_message' => 'TaintedHtml', ], + 'taintSpecializedMethodForAnonymousInstance' => [ + 'isUnsafe();', + 'error_message' => 'TaintedHtml', + ], + 'taintSpecializedMethodForStubMadeInstance' => [ + 'isUnsafe();', + 'error_message' => 'TaintedHtml', + ], 'doTaintSpecializedInstanceProperty' => [ '