danog/psalm
1
0
Fork 0
mirror of https://github.com/danog/psalm.git synced 2025-01-21 21:31:13 +01:00
Code Issues Projects Releases Wiki Activity

Fix erroneous return type resolution

Browse Source
This commit is contained in:
Brown 2019-10-14 17:10:30 -04:00
parent 3dc96edf08
commit 5e649f684c
6 changed files with 195 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -888,6 +888,30 @@ class FunctionCallAnalyzer extends \Psalm\Internal\Analyzer\Statements\Expressio
}
}
if ($codebase->taint
&& $function_id
&& $in_call_map
&& $codebase->functions->isCallMapFunctionPure($codebase, $function_id, $stmt->args)
&& isset($stmt->inferredType)
) {
if ($function_id === 'substr' && isset($stmt->args[0])) {
$stmt->inferredType->sources = $stmt->args[0]->value->inferredType->sources ?? null;
$stmt->inferredType->tainted = $stmt->args[0]->value->inferredType->tainted ?? null;
}
if (($function_id === 'str_replace' || $function_id === 'preg_replace')
&& count($stmt->args) >= 3
) {
$stmt->inferredType->sources = array_merge(
$stmt->args[1]->value->inferredType->sources ?? [],
$stmt->args[2]->value->inferredType->sources ?? []
);
$stmt->inferredType->tainted = ($stmt->args[1]->value->inferredType->tainted ?? 0)
| ($stmt->args[2]->value->inferredType->tainted ?? 0);
}
}
return null;
}
}

26
src/Psalm/Internal/Analyzer/Statements/Expression/Call/MethodCallAnalyzer.php
View File

@ -1352,32 +1352,6 @@ class MethodCallAnalyzer extends \Psalm\Internal\Analyzer\Statements\Expression\
);
}
}
if ($return_type_candidate && $codebase->taint && $method_id) {
if ($method_storage && $method_storage->pure) {
$code_location = new CodeLocation($statements_analyzer->getSource(), $stmt);
$method_source = new Source(
strtolower(
$method_id
. '-' . $code_location->file_name
. ':' . $code_location->raw_file_start
),
new CodeLocation($source, $stmt->name)
);
} else {
$method_source = new Source(
strtolower($method_id),
new CodeLocation($source, $stmt->name)
);
}
if ($tainted_source = $codebase->taint->hasPreviousSource($method_source)) {
$return_type_candidate->tainted = $tainted_source->taint;
$return_type_candidate->sources = [$method_source];
$method_source->taint = $tainted_source->taint;
}
}
}
}
}

21
src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticCallAnalyzer.php
View File

@ -1019,26 +1019,7 @@ class StaticCallAnalyzer extends \Psalm\Internal\Analyzer\Statements\Expression\
);
}
if ($tainted_source = $codebase->taint->hasPreviousSource($method_source, $suffixes)) {
$return_type_candidate->tainted = $tainted_source->taint;
if ($suffixes !== null) {
$specialized_sources = [];
foreach ($suffixes as $suffix) {
$specialized_sources[] = new Source(
$method_source->id . '-' . $suffix,
$method_source->code_location,
$tainted_source->taint
);
}
$return_type_candidate->sources = $specialized_sources;
} else {
$return_type_candidate->sources = [$method_source];
$method_source->taint = $tainted_source->taint;
}
}
$return_type_candidate->sources = [$method_source];
}
if (isset($stmt->inferredType)) {

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

@ -2867,7 +2867,7 @@ class CallAnalyzer
$child_sink = null;
if (($function_param->sink || ($child_sink = $codebase->taint->hasPreviousSink($method_sink)))
if (($function_param->sink || ($child_sink = $codebase->taint->hasPreviousSink($method_sink, $suffixes)))
&& !in_array('TaintedInput', $statements_analyzer->getSuppressedIssues())
&& $input_type->sources
) {
@ -2986,6 +2986,8 @@ class CallAnalyzer
);
}
$method_source->taint = $input_type->tainted ?: 0;
$method_source->parents = [$previous_source ?: $type_source];
$codebase->taint->addSources(

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

@ -108,10 +108,11 @@ class Taint
public function hasPreviousSource(Source $source, ?array &$suffixes = null) : ?Source
{
if (isset($this->specializations[$source->id])) {
$suffixes = $this->specializations[$source->id];
$candidate_suffixes = $this->specializations[$source->id];
foreach ($suffixes as $suffix) {
foreach ($candidate_suffixes as $suffix) {
if (isset(self::$previous_sources[$source->id . '-' . $suffix])) {
$suffixes = [$suffix];
return self::$previous_sources[$source->id . '-' . $suffix];
}
}
@ -125,7 +126,7 @@ class Taint
public function addSpecialization(string $base_id, string $suffix) : void
{
if (isset($this->specializations[$base_id])) {
if (!\in_array($suffix, $this->specializations)) {
if (!\in_array($suffix, $this->specializations[$base_id])) {
$this->specializations[$base_id][] = $suffix;
}
} else {

163
tests/TaintTest.php
View File

@ -859,4 +859,167 @@ class TaintTest extends TestCase
$this->analyzeFile('somefile.php', new Context());
}
public function testTaintOnSubstrCall() : void
{
$this->expectException(\Psalm\Exception\CodeException::class);
$this->expectExceptionMessage('TaintedInput');
$this->project_analyzer->trackTaintedInputs();
$this->addFile(
'somefile.php',
'<?php
class U {
/** @psalm-pure */
public static function shorten(string $s) : string {
return substr($s, 0, 15);
}
}
class V {}
class O1 {
public string $s;
public function __construct() {
$this->s = (string) $_GET["FOO"];
}
}
class V1 extends V {
public function foo(O1 $o) : void {
echo U::shorten($o->s);
}
}'
);
$this->analyzeFile('somefile.php', new Context());
}
public function testTaintOnStrReplaceCall() : void
{
$this->expectException(\Psalm\Exception\CodeException::class);
$this->expectExceptionMessage('TaintedInput');
$this->project_analyzer->trackTaintedInputs();
$this->addFile(
'somefile.php',
'<?php
class U {
/** @psalm-pure */
public static function shorten(string $s) : string {
return str_replace("foo", "bar", $s);
}
}
class V {}
class O1 {
public string $s;
public function __construct() {
$this->s = (string) $_GET["FOO"];
}
}
class V1 extends V {
public function foo(O1 $o) : void {
echo U::shorten($o->s);
}
}'
);
$this->analyzeFile('somefile.php', new Context());
}
public function testTaintOnPregReplaceCall() : void
{
$this->expectException(\Psalm\Exception\CodeException::class);
$this->expectExceptionMessage('TaintedInput');
$this->project_analyzer->trackTaintedInputs();
$this->addFile(
'somefile.php',
'<?php
class U {
/** @psalm-pure */
public static function shorten(string $s) : string {
return preg_replace("/foo/", "bar", $s);
}
}
class V {}
class O1 {
public string $s;
public function __construct() {
$this->s = (string) $_GET["FOO"];
}
}
class V1 extends V {
public function foo(O1 $o) : void {
echo U::shorten($o->s);
}
}'
);
$this->analyzeFile('somefile.php', new Context());
}
public function testNoTaintsOnSimilarPureCall() : void
{
$this->project_analyzer->trackTaintedInputs();
$this->addFile(
'somefile.php',
'<?php
class U {
/** @psalm-pure */
public static function shorten(string $s) : string {
return substr($s, 0, 15);
}
/** @psalm-pure */
public static function escape(string $s) : string {
return htmlentities($s);
}
}
class O1 {
public string $s;
public function __construct(string $s) {
$this->s = $s;
}
}
class O2 {
public string $t;
public function __construct() {
$this->t = (string) $_GET["FOO"];
}
}
class V1 {
public function foo() : void {
$o = new O1((string) $_GET["FOO"]);
echo U::escape(U::shorten($o->s));
}
}
class V2 {
public function foo(O2 $o) : void {
echo U::shorten(U::escape($o->t));
}
}'
);
$this->analyzeFile('somefile.php', new Context());
}
}