mirror of
https://github.com/danog/psalm.git
synced 2024-11-26 20:34:47 +01:00
Improve taint tracking during scanning phase
This commit is contained in:
parent
63c3678ae5
commit
8632cdb3cd
@ -527,6 +527,10 @@ class CommentAnalyzer
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (isset($parsed_docblock['specials']['psalm-specialize-call'])) {
|
||||||
|
$info->specialize_call = true;
|
||||||
|
}
|
||||||
|
|
||||||
if (isset($parsed_docblock['specials']['global'])) {
|
if (isset($parsed_docblock['specials']['global'])) {
|
||||||
foreach ($parsed_docblock['specials']['global'] as $offset => $global) {
|
foreach ($parsed_docblock['specials']['global'] as $offset => $global) {
|
||||||
$line_parts = self::splitDocLine($global);
|
$line_parts = self::splitDocLine($global);
|
||||||
|
@ -554,7 +554,7 @@ class ArgumentsAnalyzer
|
|||||||
$context,
|
$context,
|
||||||
$class_generic_params,
|
$class_generic_params,
|
||||||
$template_result,
|
$template_result,
|
||||||
$function_storage ? $function_storage->pure : true,
|
$function_storage ? $function_storage->specialize_call : true,
|
||||||
$in_call_map
|
$in_call_map
|
||||||
) === false) {
|
) === false) {
|
||||||
return false;
|
return false;
|
||||||
|
@ -411,6 +411,8 @@ class Analyzer
|
|||||||
|
|
||||||
$file_reference_provider = $codebase->file_reference_provider;
|
$file_reference_provider = $codebase->file_reference_provider;
|
||||||
|
|
||||||
|
$codebase->taint = new \Psalm\Internal\Codebase\Taint();
|
||||||
|
|
||||||
$file_reference_provider->setNonMethodReferencesToClasses([]);
|
$file_reference_provider->setNonMethodReferencesToClasses([]);
|
||||||
$file_reference_provider->setCallingMethodReferencesToClassMembers([]);
|
$file_reference_provider->setCallingMethodReferencesToClassMembers([]);
|
||||||
$file_reference_provider->setFileReferencesToClassMembers([]);
|
$file_reference_provider->setFileReferencesToClassMembers([]);
|
||||||
|
@ -54,7 +54,8 @@ use function substr;
|
|||||||
* diff_map:array<string, array<int, array{0:int, 1:int, 2:int, 3:int}>>,
|
* diff_map:array<string, array<int, array{0:int, 1:int, 2:int, 3:int}>>,
|
||||||
* classlike_storage:array<string, \Psalm\Storage\ClassLikeStorage>,
|
* classlike_storage:array<string, \Psalm\Storage\ClassLikeStorage>,
|
||||||
* file_storage:array<string, \Psalm\Storage\FileStorage>,
|
* file_storage:array<string, \Psalm\Storage\FileStorage>,
|
||||||
* new_file_content_hashes: array<string, string>
|
* new_file_content_hashes: array<string, string>,
|
||||||
|
* taint_data: ?\Psalm\Internal\Codebase\Taint
|
||||||
* }
|
* }
|
||||||
*/
|
*/
|
||||||
|
|
||||||
@ -430,6 +431,7 @@ class Scanner
|
|||||||
'new_file_content_hashes' => $statements_provider->parser_cache_provider
|
'new_file_content_hashes' => $statements_provider->parser_cache_provider
|
||||||
? $statements_provider->parser_cache_provider->getNewFileContentHashes()
|
? $statements_provider->parser_cache_provider->getNewFileContentHashes()
|
||||||
: [],
|
: [],
|
||||||
|
'taint_data' => $codebase->taint,
|
||||||
];
|
];
|
||||||
}
|
}
|
||||||
);
|
);
|
||||||
@ -452,6 +454,9 @@ class Scanner
|
|||||||
$this->codebase->statements_provider->addDiffMap(
|
$this->codebase->statements_provider->addDiffMap(
|
||||||
$pool_data['diff_map']
|
$pool_data['diff_map']
|
||||||
);
|
);
|
||||||
|
if ($this->codebase->taint && $pool_data['taint_data']) {
|
||||||
|
$this->codebase->taint->addThreadData($pool_data['taint_data']);
|
||||||
|
}
|
||||||
|
|
||||||
$this->codebase->file_storage_provider->addMore($pool_data['file_storage']);
|
$this->codebase->file_storage_provider->addMore($pool_data['file_storage']);
|
||||||
$this->codebase->classlike_storage_provider->addMore($pool_data['classlike_storage']);
|
$this->codebase->classlike_storage_provider->addMore($pool_data['classlike_storage']);
|
||||||
|
@ -38,9 +38,6 @@ class Taint
|
|||||||
/** @var array<string, array<string, array{array<string>, array<string>}>> */
|
/** @var array<string, array<string, array{array<string>, array<string>}>> */
|
||||||
private $forward_edges = [];
|
private $forward_edges = [];
|
||||||
|
|
||||||
/** @var array<string, array<string, array{array<string>, array<string>}>> */
|
|
||||||
private $backward_edges = [];
|
|
||||||
|
|
||||||
/** @var array<string, array<string, true>> */
|
/** @var array<string, array<string, true>> */
|
||||||
private $specialized_calls = [];
|
private $specialized_calls = [];
|
||||||
|
|
||||||
@ -77,7 +74,6 @@ class Taint
|
|||||||
$to_id = $to->id;
|
$to_id = $to->id;
|
||||||
|
|
||||||
$this->forward_edges[$from_id][$to_id] = [$added_taints, $removed_taints];
|
$this->forward_edges[$from_id][$to_id] = [$added_taints, $removed_taints];
|
||||||
$this->backward_edges[$to_id][$from_id] = [$added_taints, $removed_taints];
|
|
||||||
}
|
}
|
||||||
|
|
||||||
public function getPredecessorPath(Taintable $source) : string
|
public function getPredecessorPath(Taintable $source) : string
|
||||||
@ -137,21 +133,7 @@ class Taint
|
|||||||
if (!isset($this->forward_edges[$key])) {
|
if (!isset($this->forward_edges[$key])) {
|
||||||
$this->forward_edges[$key] = $map;
|
$this->forward_edges[$key] = $map;
|
||||||
} else {
|
} else {
|
||||||
$this->forward_edges[$key] = \array_merge(
|
$this->forward_edges[$key] += $map;
|
||||||
$this->forward_edges[$key],
|
|
||||||
$map
|
|
||||||
);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
foreach ($taint->backward_edges as $key => $map) {
|
|
||||||
if (!isset($this->backward_edges[$key])) {
|
|
||||||
$this->backward_edges[$key] = $map;
|
|
||||||
} else {
|
|
||||||
$this->backward_edges[$key] = \array_merge(
|
|
||||||
$this->backward_edges[$key],
|
|
||||||
$map
|
|
||||||
);
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@ -159,12 +141,11 @@ class Taint
|
|||||||
public function connectSinksAndSources() : void
|
public function connectSinksAndSources() : void
|
||||||
{
|
{
|
||||||
$visited_source_ids = [];
|
$visited_source_ids = [];
|
||||||
//$visited_sink_ids = [];
|
|
||||||
|
|
||||||
$sources = $this->sources;
|
$sources = $this->sources;
|
||||||
$sinks = $this->sinks;
|
$sinks = $this->sinks;
|
||||||
|
|
||||||
for ($i = 0; count($sinks) && count($sources) && $i < 10; $i++) {
|
for ($i = 0; count($sinks) && count($sources) && $i < 20; $i++) {
|
||||||
$new_sources = [];
|
$new_sources = [];
|
||||||
|
|
||||||
foreach ($sources as $source) {
|
foreach ($sources as $source) {
|
||||||
@ -242,83 +223,6 @@ class Taint
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
|
||||||
$new_sinks = [];
|
|
||||||
|
|
||||||
foreach ($sinks as $sink) {
|
|
||||||
$sink_taints = $sink->taints;
|
|
||||||
\sort($sink_taints);
|
|
||||||
|
|
||||||
$visited_sink_ids[$sink->id][implode(',', $sink_taints)] = true;
|
|
||||||
|
|
||||||
if (!isset($this->backward_edges[$sink->id])) {
|
|
||||||
continue;
|
|
||||||
}
|
|
||||||
|
|
||||||
foreach ($this->backward_edges[$sink->id] as $from_id => [$added_taints, $removed_taints]) {
|
|
||||||
if (!isset($this->nodes[$from_id])) {
|
|
||||||
continue;
|
|
||||||
}
|
|
||||||
|
|
||||||
$new_taints = \array_unique(
|
|
||||||
\array_diff(
|
|
||||||
\array_merge($sink_taints, $added_taints),
|
|
||||||
$removed_taints
|
|
||||||
)
|
|
||||||
);
|
|
||||||
|
|
||||||
\sort($new_taints);
|
|
||||||
|
|
||||||
$destination_node = $this->nodes[$from_id];
|
|
||||||
|
|
||||||
if (isset($visited_sink_ids[$from_id][implode(',', $new_taints)])) {
|
|
||||||
continue;
|
|
||||||
}
|
|
||||||
|
|
||||||
if (isset($sources[$from_id])) {
|
|
||||||
$matching_taints = array_intersect($sources[$from_id]->taints, $new_taints);
|
|
||||||
|
|
||||||
if ($matching_taints) {
|
|
||||||
if (IssueBuffer::accepts(
|
|
||||||
new TaintedInput(
|
|
||||||
'Detected taints ' . implode(', ', $matching_taints)
|
|
||||||
. ' in path: ' . $this->getPredecessorPath($sources[$from_id])
|
|
||||||
. ' -> ' . $this->getSuccessorPath($sink),
|
|
||||||
$sink->code_location
|
|
||||||
)
|
|
||||||
)) {
|
|
||||||
// fall through
|
|
||||||
}
|
|
||||||
}
|
|
||||||
} elseif (isset($new_sources[$from_id])) {
|
|
||||||
$matching_taints = array_intersect($new_sources[$from_id]->taints, $new_taints);
|
|
||||||
|
|
||||||
if ($matching_taints) {
|
|
||||||
if (IssueBuffer::accepts(
|
|
||||||
new TaintedInput(
|
|
||||||
'Detected taints ' . implode(', ', $matching_taints)
|
|
||||||
. ' in path: ' . $this->getPredecessorPath($new_sources[$from_id])
|
|
||||||
. ' -> ' . $this->getSuccessorPath($sink),
|
|
||||||
$sink->code_location
|
|
||||||
)
|
|
||||||
)) {
|
|
||||||
// fall through
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
$new_destination = clone $this->nodes[$from_id];
|
|
||||||
$new_destination->taints = $new_taints;
|
|
||||||
$new_destination->previous = $sink;
|
|
||||||
$new_destination->specialized_calls = $source->specialized_calls;
|
|
||||||
|
|
||||||
$new_sinks[$from_id] = $new_destination;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
$sinks = $new_sinks;
|
|
||||||
*/
|
|
||||||
|
|
||||||
$sources = $new_sources;
|
$sources = $new_sources;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -629,6 +629,8 @@ function preg_quote(string $str, ?string $delimiter = null) : string {}
|
|||||||
/**
|
/**
|
||||||
* @psalm-pure
|
* @psalm-pure
|
||||||
*
|
*
|
||||||
|
* @param string|int|float $args
|
||||||
|
*
|
||||||
* @psalm-flow ($format, $args) -> return
|
* @psalm-flow ($format, $args) -> return
|
||||||
*/
|
*/
|
||||||
function sprintf(string $format, ...$args) : string {}
|
function sprintf(string $format, ...$args) : string {}
|
||||||
|
@ -566,6 +566,85 @@ class TaintTest extends TestCase
|
|||||||
$this->analyzeFile('somefile.php', new Context());
|
$this->analyzeFile('somefile.php', new Context());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @return void
|
||||||
|
*/
|
||||||
|
public function testSpecializedCoreFunctionCall()
|
||||||
|
{
|
||||||
|
$this->project_analyzer->trackTaintedInputs();
|
||||||
|
|
||||||
|
$this->addFile(
|
||||||
|
'somefile.php',
|
||||||
|
'<?php
|
||||||
|
$a = (string) $_GET["user_id"];
|
||||||
|
|
||||||
|
echo print_r([], true);
|
||||||
|
|
||||||
|
$b = print_r($a, true);'
|
||||||
|
);
|
||||||
|
|
||||||
|
$this->analyzeFile('somefile.php', new Context());
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @return void
|
||||||
|
*/
|
||||||
|
public function testTaintedInputFromPropertyViaMixin()
|
||||||
|
{
|
||||||
|
$this->expectException(\Psalm\Exception\CodeException::class);
|
||||||
|
$this->expectExceptionMessage('TaintedInput');
|
||||||
|
|
||||||
|
$this->project_analyzer->trackTaintedInputs();
|
||||||
|
|
||||||
|
$this->addFile(
|
||||||
|
'somefile.php',
|
||||||
|
'<?php
|
||||||
|
class A {
|
||||||
|
public string $userId;
|
||||||
|
|
||||||
|
public function __construct() {
|
||||||
|
$this->userId = (string) $_GET["user_id"];
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/** @mixin A */
|
||||||
|
class B {
|
||||||
|
private A $a;
|
||||||
|
|
||||||
|
public function __construct(A $a) {
|
||||||
|
$this->a = $a;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function __get(string $name) {
|
||||||
|
return $this->a->$name;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
class C {
|
||||||
|
private B $b;
|
||||||
|
|
||||||
|
public function __construct(B $b) {
|
||||||
|
$this->b = $b;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getAppendedUserId() : string {
|
||||||
|
return "aaaa" . $this->b->userId;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function doDelete(PDO $pdo) : void {
|
||||||
|
$userId = $this->getAppendedUserId();
|
||||||
|
$this->deleteUser($pdo, $userId);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function deleteUser(PDO $pdo, string $userId) : void {
|
||||||
|
$pdo->exec("delete from users where user_id = " . $userId);
|
||||||
|
}
|
||||||
|
}'
|
||||||
|
);
|
||||||
|
|
||||||
|
$this->analyzeFile('somefile.php', new Context());
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @return void
|
* @return void
|
||||||
*/
|
*/
|
||||||
|
Loading…
Reference in New Issue
Block a user