1
0
mirror of https://github.com/danog/psalm.git synced 2024-11-30 04:39:00 +01:00

[BUGFIX] Continue processing psalm-flow graph after first taint sink (#5832)

Related: #5830
This commit is contained in:
Oliver Hader 2021-05-26 22:04:22 +02:00 committed by GitHub
parent 4898cd262e
commit b259296457
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 89 additions and 8 deletions

View File

@ -189,6 +189,7 @@ class TaintFlowGraph extends DataFlowGraph
\ksort($this->specializations);
\ksort($this->forward_edges);
// reprocess resolved descendants up to a maximum nesting level of 40
for ($i = 0; count($sinks) && count($sources) && $i < 40; $i++) {
$new_sources = [];
@ -434,8 +435,6 @@ class TaintFlowGraph extends DataFlowGraph
IssueBuffer::accepts($issue);
}
continue;
}
}

View File

@ -2,14 +2,14 @@
namespace Psalm\Tests;
use Psalm\Context;
use Psalm\Internal\Analyzer\IssueData;
use Psalm\IssueBuffer;
use const DIRECTORY_SEPARATOR;
class TaintTest extends TestCase
{
/**
* @dataProvider providerValidCodeParse
*
*
*/
public function testValidCode(string $code): void
{
@ -36,8 +36,6 @@ class TaintTest extends TestCase
/**
* @dataProvider providerInvalidCodeParse
*
*
*/
public function testInvalidCode(string $code, string $error_message): void
{
@ -2165,4 +2163,84 @@ class TaintTest extends TestCase
*/
];
}
/**
* @param string $code
* @param list<string> $expectedIssuesTypes
* @test
* @dataProvider multipleTaintIssuesAreDetectedDataProvider
*/
public function multipleTaintIssuesAreDetected(string $code, array $expectedIssuesTypes): void
{
if (\strpos($this->getTestName(), 'SKIPPED-') !== false) {
$this->markTestSkipped();
}
if (\strtoupper(\substr(\PHP_OS, 0, 3)) === 'WIN') {
$this->markTestSkipped('Skip taint tests in Windows for now');
}
// disables issue exceptions - we need all, not just the first
$this->testConfig->throw_exception = false;
$filePath = self::$src_dir_path . 'somefile.php';
$this->addFile($filePath, $code);
$this->project_analyzer->trackTaintedInputs();
$this->analyzeFile($filePath, new Context(), false);
$actualIssueTypes = \array_map(
function (IssueData $issue): string {
return $issue->type;
},
IssueBuffer::getIssuesDataForFile($filePath)
);
self::assertSame($expectedIssuesTypes, $actualIssueTypes);
}
/**
* @return array<string, array{0: string, expectedIssueTypes: list<string>}>
*/
public function multipleTaintIssuesAreDetectedDataProvider(): array
{
return [
'taintSinkFlow' => [
'<?php
/**
* @param string $value
* @return string
*
* @psalm-flow ($value) -> return
* @psalm-taint-sink html $value
*/
function process(string $value): string {}
$data = process((string)($_GET["inject"] ?? ""));
exec($data);
',
'expectedIssueTypes' => ['TaintedHtml', 'TaintedShell'],
],
'taintSinkCascade' => [
'<?php
function triggerHtml(string $value): string
{
echo $value;
return $value;
}
function triggerShell(string $value): string
{
exec($value);
return $value;
}
function triggerFile(string $value): string
{
file_get_contents($value);
return $value;
}
$value = (string)($_GET["inject"] ?? "");
$value = triggerHtml($value);
$value = triggerShell($value);
$value = triggerFile($value);
',
'expectedIssueTypes' => ['TaintedHtml', 'TaintedShell', 'TaintedFile'],
]
];
}
}

View File

@ -27,6 +27,9 @@ class TestCase extends BaseTestCase
/** @var Provider\FakeFileProvider */
protected $file_provider;
/** @var Config */
protected $testConfig;
public static function setUpBeforeClass() : void
{
ini_set('memory_limit', '-1');
@ -56,7 +59,7 @@ class TestCase extends BaseTestCase
$this->file_provider = new \Psalm\Tests\Internal\Provider\FakeFileProvider();
$config = $this->makeConfig();
$this->testConfig = $this->makeConfig();
$providers = new Providers(
$this->file_provider,
@ -64,7 +67,7 @@ class TestCase extends BaseTestCase
);
$this->project_analyzer = new ProjectAnalyzer(
$config,
$this->testConfig,
$providers
);
@ -73,6 +76,7 @@ class TestCase extends BaseTestCase
public function tearDown() : void
{
unset($this->project_analyzer, $this->file_provider, $this->testConfig);
RuntimeCaches::clearAll();
}