danog/psalm
1
0
Fork 0
mirror of https://github.com/danog/psalm.git synced 2024-11-27 12:55:26 +01:00
Code Issues Projects Releases Wiki Activity

Fix backtick analysis

Browse Source
This commit is contained in:
Daniil Gentili 2023-11-26 13:12:11 +01:00
parent b654545aa0
commit 35f194e9e8
3 changed files with 41 additions and 80 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

97
src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php
View File

@ -13,7 +13,6 @@ use Psalm\Internal\Analyzer\Statements\Expression\AssignmentAnalyzer;
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\UnaryPlusMinusAnalyzer;
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 @@ final class ExpressionAnalyzer
}
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_) {

8
tests/ExpressionTest.php
View File

@ -51,6 +51,14 @@ class ExpressionTest extends TestCase
'$a===' => 'array{9223372036854775806: 0, 9223372036854775807: 1}',
],
];
yield 'shellExecConcatInt' => [
'code' => <<<'PHP'
<?php
$a = 123;
/** @psalm-suppress ForbiddenCode */
`ls $a`;
PHP,
];
}
/**

16
tests/TaintTest.php
View File

@ -2306,6 +2306,22 @@ class TaintTest extends TestCase
',
'error_message' => 'TaintedShell',
],
'shellExecBacktickConcat' => [
'code' => '<?php
$input = $_GET["input"];
$x = `ls $input`;
',
'error_message' => 'TaintedShell',
],
'shellExecBacktickConcatInt' => [
'code' => '<?php
$input = (int) $_GET["input"];
$x = `ls /path/$input`;
',
'error_message' => '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.