danog/psalm
1
0
Fork 0
mirror of https://github.com/danog/psalm.git synced 2024-11-30 04:39:00 +01:00
Code Issues Projects Releases Wiki Activity

Improve property location resolution

Browse Source
This commit is contained in:
Brown 2020-05-21 23:43:13 -04:00
parent 187b944680
commit 63c3678ae5
6 changed files with 67 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/PropertyAssignmentAnalyzer.php
View File

@ -1080,18 +1080,29 @@ class PropertyAssignmentAnalyzer
$code_location = new CodeLocation($statements_analyzer->getSource(), $stmt);
$property_node = new TaintNode(
$property_id,
$localized_property_node = new TaintNode(
$property_id . '-' . $code_location->file_name . ':' . $code_location->raw_file_start,
$property_id,
$code_location,
null
);
$codebase->taint->addTaintNode($localized_property_node);
$property_node = new TaintNode(
$property_id,
$property_id,
null,
null
);
$codebase->taint->addTaintNode($property_node);
$codebase->taint->addPath($localized_property_node, $property_node);
if ($assignment_value_type->parent_nodes) {
foreach ($assignment_value_type->parent_nodes as $parent_node) {
$codebase->taint->addPath($parent_node, $property_node);
$codebase->taint->addPath($parent_node, $localized_property_node);
}
}
}

4
src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php
View File

@ -994,6 +994,10 @@ class FunctionCallAnalyzer extends CallAnalyzer
if ($codebase->taint && $function_storage && $function_storage->return_source_params && $stmt_type) {
foreach ($function_storage->return_source_params as $i) {
if (!isset($stmt->args[$i])) {
continue;
}
$arg_location = new CodeLocation(
$statements_analyzer->getSource(),
$stmt->args[$i]->value

23
src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/PropertyFetchAnalyzer.php
View File

@ -1039,16 +1039,29 @@ class PropertyFetchAnalyzer
$codebase = $statements_analyzer->getCodebase();
if ($codebase->taint) {
$property_source = new TaintNode(
$code_location = new CodeLocation($statements_analyzer, $stmt->name);
$localized_property_node = new TaintNode(
$property_id . '-' . $code_location->file_name . ':' . $code_location->raw_file_start,
$property_id,
$property_id,
new CodeLocation($statements_analyzer, $stmt->name),
$code_location,
null
);
$codebase->taint->addTaintNode($property_source);
$codebase->taint->addTaintNode($localized_property_node);
$type->parent_nodes = [$property_source];
$property_node = new TaintNode(
$property_id,
$property_id,
null,
null
);
$codebase->taint->addTaintNode($property_node);
$codebase->taint->addPath($property_node, $localized_property_node);
$type->parent_nodes = [$localized_property_node];
}
}

65
src/Psalm/Internal/Codebase/Taint.php
View File

@ -27,48 +27,39 @@ use function array_intersect;
class Taint
{
/** @var array<string, Source> */
private static $sources = [];
private $sources = [];
/** @var array<string, TaintNode> */
private static $nodes = [];
private $nodes = [];
/** @var array<string, Sink> */
private static $sinks = [];
private $sinks = [];
/** @var array<string, array<string, array{array<string>, array<string>}>> */
private static $forward_edges = [];
private $forward_edges = [];
/** @var array<string, array<string, array{array<string>, array<string>}>> */
private static $backward_edges = [];
private $backward_edges = [];
/** @var array<string, array<string, true>> */
private static $specialized_calls = [];
public function __construct()
{
self::$sinks = [];
self::$sources = [];
self::$nodes = [];
self::$forward_edges = [];
self::$backward_edges = [];
}
private $specialized_calls = [];
public function addSource(Source $node) : void
{
self::$sources[$node->id] = $node;
$this->sources[$node->id] = $node;
}
public function addSink(Sink $node) : void
{
self::$sinks[$node->id] = $node;
$this->sinks[$node->id] = $node;
}
public function addTaintNode(TaintNode $node) : void
{
self::$nodes[$node->id] = $node;
$this->nodes[$node->id] = $node;
if ($node->unspecialized_id && $node->specialization_key) {
self::$specialized_calls[$node->specialization_key][$node->unspecialized_id] = true;
$this->specialized_calls[$node->specialization_key][$node->unspecialized_id] = true;
}
}
@ -85,8 +76,8 @@ class Taint
$from_id = $from->id;
$to_id = $to->id;
self::$forward_edges[$from_id][$to_id] = [$added_taints, $removed_taints];
self::$backward_edges[$to_id][$from_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
@ -170,8 +161,8 @@ class Taint
$visited_source_ids = [];
//$visited_sink_ids = [];
$sources = self::$sources;
$sinks = self::$sinks;
$sources = $this->sources;
$sinks = $this->sinks;
for ($i = 0; count($sinks) && count($sources) && $i < 10; $i++) {
$new_sources = [];
@ -182,29 +173,29 @@ class Taint
$visited_source_ids[$source->id][implode(',', $source_taints)] = true;
if (!isset(self::$forward_edges[$source->id])) {
if (!isset($this->forward_edges[$source->id])) {
$source = clone $source;
if ($source->specialization_key && isset(self::$specialized_calls[$source->specialization_key])) {
if ($source->specialization_key && isset($this->specialized_calls[$source->specialization_key])) {
$source->specialized_calls[$source->specialization_key]
= self::$specialized_calls[$source->specialization_key];
= $this->specialized_calls[$source->specialization_key];
$source->id = substr($source->id, 0, -strlen($source->specialization_key) - 1);
} else {
foreach ($source->specialized_calls as $key => $map) {
if (isset($map[$source->id]) && isset(self::$forward_edges[$source->id . '-' . $key])) {
if (isset($map[$source->id]) && isset($this->forward_edges[$source->id . '-' . $key])) {
$source->id = $source->id . '-' . $key;
}
}
}
if (!isset(self::$forward_edges[$source->id])) {
if (!isset($this->forward_edges[$source->id])) {
continue;
}
}
foreach (self::$forward_edges[$source->id] as $to_id => [$added_taints, $removed_taints]) {
if (!isset(self::$nodes[$to_id])) {
foreach ($this->forward_edges[$source->id] as $to_id => [$added_taints, $removed_taints]) {
if (!isset($this->nodes[$to_id])) {
continue;
}
@ -217,7 +208,7 @@ class Taint
\sort($new_taints);
$destination_node = self::$nodes[$to_id];
$destination_node = $this->nodes[$to_id];
if (isset($visited_source_ids[$to_id][implode(',', $new_taints)])) {
continue;
@ -232,7 +223,7 @@ class Taint
'Detected tainted ' . implode(', ', $matching_taints)
. ' in path: ' . $this->getPredecessorPath($source)
. ' -> ' . $this->getSuccessorPath($sinks[$to_id]),
$source->code_location
$sinks[$to_id]->code_location ?: $source->code_location
)
)) {
// fall through
@ -260,12 +251,12 @@ class Taint
$visited_sink_ids[$sink->id][implode(',', $sink_taints)] = true;
if (!isset(self::$backward_edges[$sink->id])) {
if (!isset($this->backward_edges[$sink->id])) {
continue;
}
foreach (self::$backward_edges[$sink->id] as $from_id => [$added_taints, $removed_taints]) {
if (!isset(self::$nodes[$from_id])) {
foreach ($this->backward_edges[$sink->id] as $from_id => [$added_taints, $removed_taints]) {
if (!isset($this->nodes[$from_id])) {
continue;
}
@ -278,7 +269,7 @@ class Taint
\sort($new_taints);
$destination_node = self::$nodes[$from_id];
$destination_node = $this->nodes[$from_id];
if (isset($visited_sink_ids[$from_id][implode(',', $new_taints)])) {
continue;
@ -316,7 +307,7 @@ class Taint
}
}
$new_destination = clone self::$nodes[$from_id];
$new_destination = clone $this->nodes[$from_id];
$new_destination->taints = $new_taints;
$new_destination->previous = $sink;
$new_destination->specialized_calls = $source->specialized_calls;

2
src/Psalm/Internal/Taint/Taintable.php
View File

@ -40,7 +40,7 @@ abstract class Taintable
string $id,
string $label,
?CodeLocation $code_location,
?string $specialization_key,
?string $specialization_key = null,
array $taints = []
) {
$this->id = $id;

4
tests/TaintTest.php
View File

@ -241,7 +241,7 @@ class TaintTest extends TestCase
public function testTaintedInputFromParam()
{
$this->expectException(\Psalm\Exception\CodeException::class);
$this->expectExceptionMessage('TaintedInput - somefile.php:13:49 - Detected tainted sql in path: $_GET (somefile.php:4:41) -> A::getUserId (somefile.php:8:41) -> A::getAppendedUserId (somefile.php:12:35) -> $userId (somefile.php:12:25) -> A::deleteUser#2 (somefile.php:13:49) -> PDO::exec#1 (somefile.php:17:36)');
$this->expectExceptionMessage('TaintedInput - somefile.php:17:36 - Detected tainted sql in path: $_GET (somefile.php:4:41) -> A::getUserId (somefile.php:8:41) -> A::getAppendedUserId (somefile.php:12:35) -> $userId (somefile.php:12:25) -> A::deleteUser#2 (somefile.php:13:49) -> PDO::exec#1 (somefile.php:17:36)');
$this->project_analyzer->trackTaintedInputs();
@ -378,7 +378,7 @@ class TaintTest extends TestCase
public function testTaintedInputToParamAlternatePath()
{
$this->expectException(\Psalm\Exception\CodeException::class);
$this->expectExceptionMessage('TaintedInput - somefile.php:7:29 - Detected tainted sql in path: $_GET (somefile.php:7:63) -> A::getAppendedUserId#1 (somefile.php:7:54) -> A::getAppendedUserId (somefile.php:11:37) -> A::deleteUser#3 (somefile.php:7:29) -> PDO::exec#1 (somefile.php:23:40)');
$this->expectExceptionMessage('TaintedInput - somefile.php:23:40 - Detected tainted sql in path: $_GET (somefile.php:7:63) -> A::getAppendedUserId#1 (somefile.php:7:54) -> A::getAppendedUserId (somefile.php:11:37) -> A::deleteUser#3 (somefile.php:7:29) -> PDO::exec#1 (somefile.php:23:40)');
$this->project_analyzer->trackTaintedInputs();