From 67b2edc3280557079ee59058b8e7589d98932ec3 Mon Sep 17 00:00:00 2001 From: Brown Date: Thu, 2 Jul 2020 11:53:44 -0400 Subject: [PATCH] Allow more things to be suppressed with @psalm-suppress TaintedInput --- .../InstancePropertyAssignmentAnalyzer.php | 6 +++++- .../Expression/AssignmentAnalyzer.php | 19 +++++++++++-------- .../Expression/Call/NewAnalyzer.php | 1 + .../Expression/Call/StaticCallAnalyzer.php | 1 + .../Expression/Fetch/ArrayFetchAnalyzer.php | 6 +++++- .../Fetch/InstancePropertyFetchAnalyzer.php | 10 +++++++--- .../Fetch/VariableFetchAnalyzer.php | 5 ++++- tests/TaintTest.php | 12 ++++++++++++ 8 files changed, 46 insertions(+), 14 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/InstancePropertyAssignmentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/InstancePropertyAssignmentAnalyzer.php index 1b5265c81..f800b4953 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/InstancePropertyAssignmentAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/InstancePropertyAssignmentAnalyzer.php @@ -1112,7 +1112,6 @@ class InstancePropertyAssignmentAnalyzer if (!$codebase->taint || !$codebase->config->trackTaintsInPath($statements_analyzer->getFilePath()) - || \in_array('TaintedInput', $statements_analyzer->getSuppressedIssues()) ) { return; } @@ -1134,6 +1133,11 @@ class InstancePropertyAssignmentAnalyzer ); if ($var_id) { + if (\in_array('TaintedInput', $statements_analyzer->getSuppressedIssues())) { + $context->vars_in_scope[$var_id]->parent_nodes = []; + return; + } + $var_node = TaintNode::getForAssignment( $var_id, $var_location diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php index 25e7c5be1..fff0c8034 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php @@ -943,20 +943,23 @@ class AssignmentAnalyzer if ($codebase->taint && $codebase->config->trackTaintsInPath($statements_analyzer->getFilePath()) - && !\in_array('TaintedInput', $statements_analyzer->getSuppressedIssues()) ) { if ($context->vars_in_scope[$var_id]->parent_nodes) { - $var_location = new CodeLocation($statements_analyzer->getSource(), $assign_var); + if (\in_array('TaintedInput', $statements_analyzer->getSuppressedIssues())) { + $context->vars_in_scope[$var_id]->parent_nodes = []; + } else { + $var_location = new CodeLocation($statements_analyzer->getSource(), $assign_var); - $new_parent_node = \Psalm\Internal\Taint\TaintNode::getForAssignment($var_id, $var_location); + $new_parent_node = \Psalm\Internal\Taint\TaintNode::getForAssignment($var_id, $var_location); - $codebase->taint->addTaintNode($new_parent_node); + $codebase->taint->addTaintNode($new_parent_node); - foreach ($context->vars_in_scope[$var_id]->parent_nodes as $parent_node) { - $codebase->taint->addPath($parent_node, $new_parent_node, '=', [], $removed_taints); + foreach ($context->vars_in_scope[$var_id]->parent_nodes as $parent_node) { + $codebase->taint->addPath($parent_node, $new_parent_node, '=', [], $removed_taints); + } + + $context->vars_in_scope[$var_id]->parent_nodes = [$new_parent_node]; } - - $context->vars_in_scope[$var_id]->parent_nodes = [$new_parent_node]; } } } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php index fd2f3a746..7156d95aa 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php @@ -572,6 +572,7 @@ class NewAnalyzer extends \Psalm\Internal\Analyzer\Statements\Expression\CallAna if ($codebase->taint && $codebase->config->trackTaintsInPath($statements_analyzer->getFilePath()) + && !\in_array('TaintedInput', $statements_analyzer->getSuppressedIssues()) && ($stmt_type = $statements_analyzer->node_data->getType($stmt)) ) { $code_location = new CodeLocation($statements_analyzer->getSource(), $stmt); diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticCallAnalyzer.php index 1542f5703..d2f0b9324 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticCallAnalyzer.php @@ -1358,6 +1358,7 @@ class StaticCallAnalyzer extends \Psalm\Internal\Analyzer\Statements\Expression\ if (!$codebase->taint || !$codebase->config->trackTaintsInPath($statements_analyzer->getFilePath()) + || \in_array('TaintedInput', $statements_analyzer->getSuppressedIssues()) ) { return; } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php index 2d760ac68..99cea4d55 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php @@ -318,8 +318,12 @@ class ArrayFetchAnalyzer && ($stmt_var_type = $statements_analyzer->node_data->getType($var)) && $stmt_var_type->parent_nodes && $codebase->config->trackTaintsInPath($statements_analyzer->getFilePath()) - && !\in_array('TaintedInput', $statements_analyzer->getSuppressedIssues()) ) { + if (\in_array('TaintedInput', $statements_analyzer->getSuppressedIssues())) { + $stmt_var_type->parent_nodes = []; + return; + } + $var_location = new CodeLocation($statements_analyzer->getSource(), $var); $new_parent_node = \Psalm\Internal\Taint\TaintNode::getForAssignment( diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/InstancePropertyFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/InstancePropertyFetchAnalyzer.php index 0f463060d..15a519985 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/InstancePropertyFetchAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/InstancePropertyFetchAnalyzer.php @@ -1076,7 +1076,6 @@ class InstancePropertyFetchAnalyzer if (!$codebase->taint || !$codebase->config->trackTaintsInPath($statements_analyzer->getFilePath()) - || \in_array('TaintedInput', $statements_analyzer->getSuppressedIssues()) ) { return; } @@ -1098,6 +1097,13 @@ class InstancePropertyFetchAnalyzer ); if ($var_id) { + $var_type = $statements_analyzer->node_data->getType($stmt->var); + + if ($var_type && \in_array('TaintedInput', $statements_analyzer->getSuppressedIssues())) { + $var_type->parent_nodes = []; + return; + } + $var_node = TaintNode::getForAssignment( $var_id, $var_location @@ -1119,8 +1125,6 @@ class InstancePropertyFetchAnalyzer . ($stmt->name instanceof PhpParser\Node\Identifier ? '-' . $stmt->name : '') ); - $var_type = $statements_analyzer->node_data->getType($stmt->var); - if ($var_type && $var_type->parent_nodes) { foreach ($var_type->parent_nodes as $parent_node) { $codebase->taint->addPath( diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/VariableFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/VariableFetchAnalyzer.php index 520887428..6418042ca 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/VariableFetchAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/VariableFetchAnalyzer.php @@ -360,7 +360,10 @@ class VariableFetchAnalyzer ) : void { $codebase = $statements_analyzer->getCodebase(); - if ($codebase->taint && $codebase->config->trackTaintsInPath($statements_analyzer->getFilePath())) { + if ($codebase->taint + && $codebase->config->trackTaintsInPath($statements_analyzer->getFilePath()) + && !\in_array('TaintedInput', $statements_analyzer->getSuppressedIssues()) + ) { if ($var_name === '$_GET' || $var_name === '$_POST' || $var_name === '$_COOKIE' diff --git a/tests/TaintTest.php b/tests/TaintTest.php index a3438a39b..71dfd8731 100644 --- a/tests/TaintTest.php +++ b/tests/TaintTest.php @@ -474,6 +474,18 @@ class TaintTest extends TestCase echo $_GET["x"]; }' ], + 'suppressTaintedAssignment' => [ + '