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 speed of taint analysis

Browse Source
This commit is contained in:
Matthew Brown 2019-10-13 20:10:31 -04:00
parent e1e2ff3e57
commit 8c6b234c2c
5 changed files with 70 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -987,7 +987,6 @@ class PropertyAssignmentAnalyzer
if ($child_sink = $codebase->taint->hasPreviousSink($method_sink)) {
if ($assignment_value_type->sources) {
$codebase->taint->addSinks(
$statements_analyzer,
\array_map(
function (Source $assignment_source) use ($child_sink) {
$new_sink = new Sink(
@ -1028,7 +1027,6 @@ class PropertyAssignmentAnalyzer
$new_source->taint = $previous_source->taint;
$codebase->taint->addSources(
$statements_analyzer,
[$new_source]
);
}

3
src/Psalm/Internal/Analyzer/Statements/Expression/CallAnalyzer.php
View File

@ -2868,6 +2868,7 @@ class CallAnalyzer
$child_sink = null;
if (($function_param->sink || ($child_sink = $codebase->taint->hasPreviousSink($method_sink)))
&& !in_array('TaintedInput', $statements_analyzer->getSuppressedIssues())
&& $input_type->sources
) {
$all_possible_sinks = [];
@ -2929,7 +2930,6 @@ class CallAnalyzer
}
$codebase->taint->addSinks(
$statements_analyzer,
$all_possible_sinks
);
}
@ -2989,7 +2989,6 @@ class CallAnalyzer
$method_source->parents = [$previous_source ?: $type_source];
$codebase->taint->addSources(
$statements_analyzer,
[$method_source]
);
}

2
src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php
View File

@ -471,7 +471,6 @@ class ReturnAnalyzer
}
$codebase->taint->addSinks(
$statements_analyzer,
$new_sinks
);
}
@ -509,7 +508,6 @@ class ReturnAnalyzer
}
$codebase->taint->addSources(
$statements_analyzer,
$new_sources
);
}

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

@ -72,6 +72,16 @@ class Taint
return self::$archived_sources[$source->id] ?? null;
}
public function hasNewOrExistingSink(Taintable $sink) : ?Sink
{
return $this->new_sinks[$sink->id] ?? self::$archived_sinks[$sink->id] ?? null;
}
public function hasNewOrExistingSource(Taintable $source) : ?Source
{
return $this->new_sources[$source->id] ?? self::$archived_sources[$source->id] ?? null;
}
/**
* @param ?array<string> $suffixes
*/
@ -127,7 +137,6 @@ class Taint
* @param array<Source> $sources
*/
public function addSources(
StatementsAnalyzer $statements_analyzer,
array $sources
) : void {
foreach ($sources as $source) {
@ -135,28 +144,8 @@ class Taint
continue;
}
if (($existing_sink = $this->hasExistingSink($source)) && $source->code_location) {
$last_location = $existing_sink;
while ($last_location->children) {
$first_child = \reset($last_location->children);
if (!$first_child->code_location) {
break;
}
$last_location = $first_child;
}
if (IssueBuffer::accepts(
new TaintedInput(
'in path ' . $this->getPredecessorPath($source)
. ' out path ' . $this->getSuccessorPath($existing_sink),
$last_location->code_location ?: $source->code_location
),
$statements_analyzer->getSuppressedIssues()
)) {
// fall through
}
if ($this->hasExistingSink($source) && $source->code_location) {
// do nothing
}
$this->new_sources[$source->id] = $source;
@ -167,7 +156,6 @@ class Taint
* @param array<Sink> $sinks
*/
public function addSinks(
StatementsAnalyzer $statements_analyzer,
array $sinks
) : void {
foreach ($sinks as $sink) {
@ -175,28 +163,8 @@ class Taint
continue;
}
if (($existing_source = $this->hasExistingSource($sink)) && $sink->code_location) {
$last_location = $sink;
while ($last_location->children) {
$first_child = \reset($last_location->children);
if (!$first_child->code_location) {
break;
}
$last_location = $first_child;
}
if (IssueBuffer::accepts(
new TaintedInput(
'in path ' . $this->getPredecessorPath($existing_source)
. ' out path ' . $this->getSuccessorPath($sink),
$last_location->code_location ?: $sink->code_location
),
$statements_analyzer->getSuppressedIssues()
)) {
// fall through
}
if ($this->hasExistingSource($sink) && $sink->code_location) {
// do nothing
}
$this->new_sinks[$sink->id] = $sink;
@ -269,6 +237,56 @@ class Taint
public function hasNewSinksAndSources() : bool
{
foreach ($this->new_sinks as $sink) {
if ($sink && ($existing_source = $this->hasNewOrExistingSource($sink)) && $sink->code_location) {
$last_location = $sink;
while ($last_location->children) {
$first_child = \reset($last_location->children);
if (!$first_child->code_location) {
break;
}
$last_location = $first_child;
}
if (IssueBuffer::accepts(
new TaintedInput(
'in path ' . $this->getPredecessorPath($existing_source)
. ' out path ' . $this->getSuccessorPath($sink),
$last_location->code_location ?: $sink->code_location
)
)) {
// fall through
}
}
}
foreach ($this->new_sources as $source) {
if ($source && ($existing_sink = $this->hasNewOrExistingSink($source)) && $source->code_location) {
$last_location = $existing_sink;
while ($last_location->children) {
$first_child = \reset($last_location->children);
if (!$first_child->code_location) {
break;
}
$last_location = $first_child;
}
if (IssueBuffer::accepts(
new TaintedInput(
'in path ' . $this->getPredecessorPath($source)
. ' out path ' . $this->getSuccessorPath($existing_sink),
$last_location->code_location ?: $source->code_location
)
)) {
// fall through
}
}
}
if (!self::$archived_sources && !$this->new_sources) {
return false;
}
@ -310,7 +328,7 @@ class Taint
) : array {
$files = [];
foreach (array_merge(self::$archived_sinks, $this->new_sinks) as $new_sink) {
foreach ($this->new_sinks as $new_sink) {
if ($new_sink && $new_sink->code_location) {
$files = array_merge(
$reference_provider->getFilesReferencingFile($new_sink->code_location->file_path),
@ -319,7 +337,7 @@ class Taint
}
}
foreach (array_merge(self::$archived_sources, $this->new_sources) as $new_source) {
foreach ($this->new_sources as $new_source) {
if ($new_source && $new_source->code_location) {
$classlikes = $file_storage_provider->get($new_source->code_location->file_path)->classlikes_in_file;
foreach ($classlikes as $classlike) {

6
tests/TaintTest.php
View File

@ -239,7 +239,7 @@ class TaintTest extends TestCase
public function testTaintedInputFromParam()
{
$this->expectException(\Psalm\Exception\CodeException::class);
$this->expectExceptionMessage('TaintedInput - somefile.php:17:36 - in path $_GET (somefile.php:4) -> a::getuserid (somefile.php:3) out path a::getuserid (somefile.php:8) -> a::getappendeduserid (somefile.php:12) -> a::deleteuser#2 (somefile.php:16) -> pdo::exec#1 (somefile.php:17)');
$this->expectExceptionMessage('TaintedInput - somefile.php:17:36 - in path $_GET (somefile.php:4) -> a::getuserid (somefile.php:3) -> a::getappendeduserid (somefile.php:7) out path a::getappendeduserid (somefile.php:12) -> a::deleteuser#2 (somefile.php:16) -> pdo::exec#1 (somefile.php:17)');
$this->project_analyzer->trackTaintedInputs();
@ -376,7 +376,7 @@ class TaintTest extends TestCase
public function testTaintedInputToParamAlternatePath()
{
$this->expectException(\Psalm\Exception\CodeException::class);
$this->expectExceptionMessage('TaintedInput - somefile.php:23:40 - in path $_GET (somefile.php:7) -> a::getappendeduserid#1 (somefile.php:7) -> a::getappendeduserid (somefile.php:11) -> a::deleteuser#3 (somefile.php:7) out path a::deleteuser#3 (somefile.php:19) -> pdo::exec#1 (somefile.php:23)');
$this->expectExceptionMessage('TaintedInput - somefile.php:23:40 - in path $_GET (somefile.php:7) -> a::getappendeduserid#1 (somefile.php:7) -> a::getappendeduserid (somefile.php:11) out path a::getappendeduserid (somefile.php:7) -> a::deleteuser#3 (somefile.php:19) -> pdo::exec#1 (somefile.php:23)');
$this->project_analyzer->trackTaintedInputs();
@ -419,7 +419,7 @@ class TaintTest extends TestCase
public function testTaintedInParentLoader()
{
$this->expectException(\Psalm\Exception\CodeException::class);
$this->expectExceptionMessage('TaintedInput - somefile.php:16:40 - in path $_GET (somefile.php:28) -> c::foo#1 (somefile.php:28) out path c::foo#1 (somefile.php:23) -> agrandchild::loadfull#1 (somefile.php:6) -> a::loadpartial#1 (somefile.php:16) -> pdo::exec#1 (somefile.php:16)');
$this->expectExceptionMessage('TaintedInput - somefile.php:16:40 - in path $_GET (somefile.php:28) -> c::foo#1 (somefile.php:28) -> agrandchild::loadfull#1 (somefile.php:24) out path agrandchild::loadfull#1 (somefile.php:6) -> a::loadpartial#1 (somefile.php:16) -> pdo::exec#1 (somefile.php:16)');
$this->project_analyzer->trackTaintedInputs();