From 13fc8a75fda558b452ef5d0048fe8f09e870d6c6 Mon Sep 17 00:00:00 2001 From: Brown Date: Tue, 23 Jun 2020 15:52:19 -0400 Subject: [PATCH] Allow taints to flow where no return type exists Fixes #3652 --- .../Expression/Call/FunctionCallAnalyzer.php | 12 ++-- .../Expression/Call/StaticCallAnalyzer.php | 34 +++++----- tests/TaintTest.php | 64 +++++++++++++++++++ 3 files changed, 88 insertions(+), 22 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php index 3aa9ed5a3..54b005126 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php @@ -359,9 +359,7 @@ class FunctionCallAnalyzer extends CallAnalyzer $context ); - if ($stmt_type) { - $statements_analyzer->node_data->setType($real_stmt, $stmt_type); - } + $statements_analyzer->node_data->setType($real_stmt, $stmt_type); if ($config->after_every_function_checks) { foreach ($config->after_every_function_checks as $plugin_fq_class_name) { @@ -897,7 +895,7 @@ class FunctionCallAnalyzer extends CallAnalyzer ?FunctionLikeStorage $function_storage, TemplateResult $template_result, Context $context - ) : ?Type\Union { + ) : Type\Union { $stmt_type = null; $config = $codebase->config; @@ -1026,7 +1024,11 @@ class FunctionCallAnalyzer extends CallAnalyzer } } - if ($function_storage && $stmt_type) { + if (!$stmt_type) { + $stmt_type = Type::getMixed(); + } + + if ($function_storage) { self::taintReturnType($statements_analyzer, $stmt, $function_id, $function_storage, $stmt_type); } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticCallAnalyzer.php index f60bb7aaa..1542f5703 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticCallAnalyzer.php @@ -1239,24 +1239,24 @@ class StaticCallAnalyzer extends \Psalm\Internal\Analyzer\Statements\Expression\ } } - if ($return_type_candidate) { - self::taintReturnType( - $statements_analyzer, - $stmt, - $method_id, - $cased_method_id, - $return_type_candidate, - $method_storage - ); + $return_type_candidate = $return_type_candidate ?: Type::getMixed(); - if ($stmt_type = $statements_analyzer->node_data->getType($stmt)) { - $statements_analyzer->node_data->setType( - $stmt, - Type::combineUnionTypes($stmt_type, $return_type_candidate) - ); - } else { - $statements_analyzer->node_data->setType($stmt, $return_type_candidate); - } + self::taintReturnType( + $statements_analyzer, + $stmt, + $method_id, + $cased_method_id, + $return_type_candidate, + $method_storage + ); + + if ($stmt_type = $statements_analyzer->node_data->getType($stmt)) { + $statements_analyzer->node_data->setType( + $stmt, + Type::combineUnionTypes($stmt_type, $return_type_candidate) + ); + } else { + $statements_analyzer->node_data->setType($stmt, $return_type_candidate); } } else { if ($stmt->name instanceof PhpParser\Node\Expr) { diff --git a/tests/TaintTest.php b/tests/TaintTest.php index 1015e1b29..370fda5d0 100644 --- a/tests/TaintTest.php +++ b/tests/TaintTest.php @@ -1775,4 +1775,68 @@ class TaintTest extends TestCase $this->analyzeFile('somefile.php', new Context()); } + + public function testTaintedFunctionWithNoTypes() : void + { + $this->expectException(\Psalm\Exception\CodeException::class); + $this->expectExceptionMessage('TaintedInput'); + + $this->project_analyzer->trackTaintedInputs(); + + $this->addFile( + 'somefile.php', + 'analyzeFile('somefile.php', new Context()); + } + + public function testTaintedStaticCallWithNoTypes() : void + { + $this->expectException(\Psalm\Exception\CodeException::class); + $this->expectExceptionMessage('TaintedInput'); + + $this->project_analyzer->trackTaintedInputs(); + + $this->addFile( + 'somefile.php', + 'analyzeFile('somefile.php', new Context()); + } + + public function testTaintedInstanceCallWithNoTypes() : void + { + $this->expectException(\Psalm\Exception\CodeException::class); + $this->expectExceptionMessage('TaintedInput'); + + $this->project_analyzer->trackTaintedInputs(); + + $this->addFile( + 'somefile.php', + 'rawinput();' + ); + + $this->analyzeFile('somefile.php', new Context()); + } }