diff --git a/dictionaries/PropertyMap.php b/dictionaries/PropertyMap.php index 7f07ea657de..c5755235e91 100644 --- a/dictionaries/PropertyMap.php +++ b/dictionaries/PropertyMap.php @@ -384,7 +384,7 @@ 'items' => 'array', ], 'phpparser\\node\\expr\\shellexec' => [ - 'parts' => 'list', + 'parts' => 'list', ], 'phpparser\\node\\matcharm' => [ 'conds' => 'null|non-empty-list', diff --git a/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php index 277787b3ef7..063277aaff6 100644 --- a/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php @@ -13,7 +13,6 @@ use Psalm\Internal\Analyzer\Statements\Expression\BinaryOpAnalyzer; use Psalm\Internal\Analyzer\Statements\Expression\BitwiseNotAnalyzer; use Psalm\Internal\Analyzer\Statements\Expression\BooleanNotAnalyzer; -use Psalm\Internal\Analyzer\Statements\Expression\Call\ArgumentAnalyzer; use Psalm\Internal\Analyzer\Statements\Expression\Call\FunctionCallAnalyzer; use Psalm\Internal\Analyzer\Statements\Expression\Call\MethodCallAnalyzer; use Psalm\Internal\Analyzer\Statements\Expression\Call\NewAnalyzer; @@ -43,20 +42,18 @@ use Psalm\Internal\Analyzer\Statements\Expression\YieldAnalyzer; use Psalm\Internal\Analyzer\Statements\Expression\YieldFromAnalyzer; use Psalm\Internal\Analyzer\StatementsAnalyzer; -use Psalm\Internal\Codebase\TaintFlowGraph; -use Psalm\Internal\DataFlow\DataFlowNode; -use Psalm\Internal\DataFlow\TaintSink; use Psalm\Internal\FileManipulation\FileManipulationBuffer; use Psalm\Internal\Type\TemplateResult; -use Psalm\Issue\ForbiddenCode; use Psalm\Issue\UnrecognizedExpression; use Psalm\Issue\UnsupportedReferenceUsage; use Psalm\IssueBuffer; +use Psalm\Node\Expr\VirtualFuncCall; +use Psalm\Node\Scalar\VirtualEncapsed; +use Psalm\Node\VirtualArg; +use Psalm\Node\VirtualName; use Psalm\Plugin\EventHandler\Event\AfterExpressionAnalysisEvent; use Psalm\Plugin\EventHandler\Event\BeforeExpressionAnalysisEvent; -use Psalm\Storage\FunctionLikeParameter; use Psalm\Type; -use Psalm\Type\TaintKind; use function get_class; use function in_array; @@ -378,80 +375,20 @@ private static function handleExpression( } if ($stmt instanceof PhpParser\Node\Expr\ShellExec) { - if ($statements_analyzer->data_flow_graph) { - $call_location = new CodeLocation($statements_analyzer->getSource(), $stmt); - - if ($statements_analyzer->data_flow_graph instanceof TaintFlowGraph) { - $sink = TaintSink::getForMethodArgument( - 'shell_exec', - 'shell_exec', - 0, - null, - $call_location, - ); - - $sink->taints = [TaintKind::INPUT_SHELL]; - - $statements_analyzer->data_flow_graph->addSink($sink); - } - - foreach ($stmt->parts as $part) { - if ($part instanceof PhpParser\Node\Expr\Variable) { - if (self::analyze($statements_analyzer, $part, $context) === false) { - break; - } - - $expr_type = $statements_analyzer->node_data->getType($part); - if ($expr_type === null) { - break; - } - - $shell_exec_param = new FunctionLikeParameter( - 'var', - false, - ); - - if (ArgumentAnalyzer::verifyType( - $statements_analyzer, - $expr_type, - Type::getString(), - null, - 'shell_exec', - null, - 0, - $call_location, - $stmt, - $context, - $shell_exec_param, - false, - null, - true, - true, - new CodeLocation($statements_analyzer, $stmt), - ) === false) { - return false; - } - - foreach ($expr_type->parent_nodes as $parent_node) { - $statements_analyzer->data_flow_graph->addPath( - $parent_node, - new DataFlowNode('variable-use', 'variable use', null), - 'variable-use', - ); - } - } - } - } - - IssueBuffer::maybeAdd( - new ForbiddenCode( - 'Use of shell_exec', - new CodeLocation($statements_analyzer->getSource(), $stmt), - ), - $statements_analyzer->getSuppressedIssues(), + $concat = new VirtualEncapsed($stmt->parts, $stmt->getAttributes()); + $virtual_call = new VirtualFuncCall(new VirtualName(['shell_exec']), [ + new VirtualArg($concat), + ], $stmt->getAttributes()); + return self::handleExpression( + $statements_analyzer, + $virtual_call, + $context, + $array_assignment, + $global_context, + $from_stmt, + $template_result, + $assigned_to_reference, ); - - return true; } if ($stmt instanceof PhpParser\Node\Expr\Print_) { diff --git a/tests/ExpressionTest.php b/tests/ExpressionTest.php index 57186daf99d..97c57c9db96 100644 --- a/tests/ExpressionTest.php +++ b/tests/ExpressionTest.php @@ -51,6 +51,14 @@ public function providerValidCodeParse(): iterable '$a===' => 'array{9223372036854775806: 0, 9223372036854775807: 1}', ], ]; + yield 'shellExecConcatInt' => [ + 'code' => <<<'PHP' + 'TaintedShell', ], + 'shellExecBacktickConcat' => [ + 'code' => ' 'TaintedShell', + ], /* // TODO: Stubs do not support this type of inference even with $this->message = $message. // Most uses of getMessage() would be with caught exceptions, so this is not representative of real code.