danog/psalm
1
0
Fork 0
mirror of https://github.com/danog/psalm.git synced 2025-01-22 05:41:20 +01:00
Code Issues Projects Releases Wiki Activity

Make taint checks more thorough

Browse Source
This commit is contained in:
Brown 2020-05-25 17:10:53 -04:00
parent 2e6fc24867
commit 7e7456c863
7 changed files with 196 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php
View File

@ -170,7 +170,7 @@ class ArgumentAnalyzer
bool $in_call_map
) {
if (!$function_param->type) {
if (!$codebase->infer_types_from_usage) {
if (!$codebase->infer_types_from_usage && !$codebase->taint) {
return;
}

2
src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php
View File

@ -770,7 +770,7 @@ class AtomicMethodCallAnalyzer extends CallAnalyzer
/** @psalm-suppress UndefinedPropertyAssignment */
$stmt->pure = true;
}
} elseif ($return_type_candidate) {
} else {
$context->vars_in_scope[$method_var_id] = $return_type_candidate;
}
}

48
src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/MethodCallReturnTypeFetcher.php
View File

@ -33,7 +33,7 @@ class MethodCallReturnTypeFetcher
array $args,
AtomicMethodCallAnalysisResult $result,
TemplateResult $template_result
) : ?Type\Union {
) : Type\Union {
$call_map_id = $declaring_method_id ?: $method_id;
$fq_class_name = $method_id->fq_class_name;
@ -149,27 +149,6 @@ class MethodCallReturnTypeFetcher
&& $codebase->classlike_storage_provider->get($static_type->value)->final
);
if ($codebase->taint && $declaring_method_id) {
$method_storage = $codebase->methods->getStorage(
$declaring_method_id
);
$node_location = new CodeLocation($statements_analyzer, $stmt);
$method_call_node = TaintNode::getForMethodReturn(
(string) $method_id,
$cased_method_id,
$node_location,
$method_storage->specialize_call ? $node_location : null
);
$codebase->taint->addTaintNode($method_call_node);
$return_type_candidate->parent_nodes = [
$method_call_node
];
}
$return_type_location = $codebase->methods->getMethodReturnTypeLocation(
$method_id,
$secondary_return_type_location
@ -201,6 +180,31 @@ class MethodCallReturnTypeFetcher
}
}
if (!$return_type_candidate) {
$return_type_candidate = $method_name === '__tostring' ? Type::getString() : Type::getMixed();
}
if ($codebase->taint && $declaring_method_id) {
$method_storage = $codebase->methods->getStorage(
$declaring_method_id
);
$node_location = new CodeLocation($statements_analyzer, $stmt);
$method_call_node = TaintNode::getForMethodReturn(
(string) $method_id,
$cased_method_id,
$node_location,
$method_storage->specialize_call ? $node_location : null
);
$codebase->taint->addTaintNode($method_call_node);
$return_type_candidate->parent_nodes = [
$method_call_node
];
}
return $return_type_candidate;
}

70
src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php
View File

@ -112,24 +112,36 @@ class ArrayFetchAnalyzer
$stmt_var_type = $statements_analyzer->node_data->getType($stmt->var);
$codebase = $statements_analyzer->getCodebase();
if ($keyed_array_var_id
&& $context->hasVariable($keyed_array_var_id)
&& !$context->vars_in_scope[$keyed_array_var_id]->possibly_undefined
&& $stmt_var_type
&& !$stmt_var_type->hasClassStringMap()
) {
$stmt_type = clone $context->vars_in_scope[$keyed_array_var_id];
$statements_analyzer->node_data->setType(
$stmt,
clone $context->vars_in_scope[$keyed_array_var_id]
$stmt_type
);
if ($array_var_id) {
self::taintArrayType(
$statements_analyzer,
$codebase,
$stmt,
$stmt_type,
$array_var_id
);
}
return true;
}
$can_store_result = false;
$codebase = $statements_analyzer->getCodebase();
if ($stmt_var_type) {
if ($stmt_var_type->isNull()) {
if (!$context->inside_isset) {
@ -191,24 +203,14 @@ class ArrayFetchAnalyzer
$statements_analyzer->node_data->setType($stmt, $stmt_type);
if ($codebase->taint) {
if ($array_var_id === '$_GET' || $array_var_id === '$_POST' || $array_var_id === '$_COOKIE') {
$taint_location = new CodeLocation($statements_analyzer->getSource(), $stmt);
$server_taint_source = new Source(
$array_var_id . ':' . $taint_location->file_name . ':' . $taint_location->raw_file_start,
$array_var_id,
$taint_location,
null,
Type\TaintKindGroup::ALL_INPUT
if ($array_var_id) {
self::taintArrayType(
$statements_analyzer,
$codebase,
$stmt,
$stmt_type,
$array_var_id
);
$codebase->taint->addSource($server_taint_source);
$stmt_type->parent_nodes = [
$server_taint_source
];
}
}
if ($context->inside_isset
@ -1583,4 +1585,32 @@ class ArrayFetchAnalyzer
return $offset_type;
}
private static function taintArrayType(
StatementsAnalyzer $statements_analyzer,
\Psalm\Codebase $codebase,
PhpParser\Node\Expr $stmt,
Type\Union $stmt_type,
string $array_var_id
) : void {
if ($codebase->taint) {
if ($array_var_id === '$_GET' || $array_var_id === '$_POST' || $array_var_id === '$_COOKIE') {
$taint_location = new CodeLocation($statements_analyzer->getSource(), $stmt);
$server_taint_source = new Source(
$array_var_id . ':' . $taint_location->file_name . ':' . $taint_location->raw_file_start,
$array_var_id,
$taint_location,
null,
Type\TaintKindGroup::ALL_INPUT
);
$codebase->taint->addSource($server_taint_source);
$stmt_type->parent_nodes = [
$server_taint_source
];
}
}
}
}

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

@ -41,6 +41,9 @@ class Taint
/** @var array<string, array<string, true>> */
private $specialized_calls = [];
/** @var array<string, array<string, true>> */
private $specializations = [];
public function addSource(Source $node) : void
{
$this->sources[$node->id] = $node;
@ -59,6 +62,7 @@ class Taint
if ($node->unspecialized_id && $node->specialization_key) {
$this->specialized_calls[$node->specialization_key][$node->unspecialized_id] = true;
$this->specializations[$node->unspecialized_id][$node->specialization_key] = true;
}
}
@ -138,6 +142,14 @@ class Taint
$this->forward_edges[$key] += $map;
}
}
foreach ($taint->specializations as $key => $map) {
if (!isset($this->specializations[$key])) {
$this->specializations[$key] = $map;
} else {
$this->specializations[$key] += $map;
}
}
}
public function connectSinksAndSources() : void
@ -164,6 +176,11 @@ class Taint
= $this->specialized_calls[$source->specialization_key];
$source->id = substr($source->id, 0, -strlen($source->specialization_key) - 1);
} elseif (isset($this->specializations[$source->id])) {
foreach ($this->specializations[$source->id] as $specialization => $_) {
// TODO: generate multiple new sources
$source->id = $source->id . '-' . $specialization;
}
} else {
foreach ($source->specialized_calls as $key => $map) {
if (isset($map[$source->id]) && isset($this->forward_edges[$source->id . '-' . $key])) {

76
tests/TaintTest.php
View File

@ -1212,4 +1212,80 @@ class TaintTest extends TestCase
$this->analyzeFile('somefile.php', new Context());
}
public function testIndirectGetAssignment() : void
{
$this->expectException(\Psalm\Exception\CodeException::class);
$this->expectExceptionMessage('TaintedInput');
$this->project_analyzer->trackTaintedInputs();
$this->addFile(
'somefile.php',
'<?php
class InputFilter {
public string $name;
public function __construct(string $name) {
$this->name = $name;
}
/**
* @psalm-specialize-call
*/
public function getArg(string $method, string $type)
{
$arg = null;
switch ($method) {
case "post":
if (isset($_POST[$this->name])) {
$arg = $_POST[$this->name];
}
break;
case "get":
if (isset($_GET[$this->name])) {
$arg = $_GET[$this->name];
}
break;
}
return $this->filterInput($type, $arg);
}
protected function filterInput(string $type, $arg)
{
// input is null
if ($arg === null) {
return null;
}
// set to null if sanitize clears arg
if ($arg === "") {
$arg = null;
}
// type casting
if ($arg !== null) {
$arg = $this->typeCastInput($type, $arg);
}
return $arg;
}
protected function typeCastInput(string $type, $arg) {
if ($type === "string") {
return (string) $arg;
}
return null;
}
}
echo (new InputFilter("hello"))->getArg("get", "string");'
);
$this->analyzeFile('somefile.php', new Context());
}
}

24
tests/Template/ClassTemplateTest.php
View File

@ -2757,6 +2757,30 @@ class ClassTemplateTest extends TestCase
/** @var Foo<Enum::TYPE_ONE> $foo */
$foo = new Foo();'
],
'extendedPropertyTypeParameterised' => [
'<?php
namespace App;
use DateTimeImmutable;
use Ds\Map;
abstract class Z
{
public function test(): void
{
$map = $this->createMap();
$date = $map->get("test");
echo $date->format("Y");
}
/**
* @return Map<string, DateTimeImmutable>
*/
abstract protected function createMap(): Map;
}'
],
];
}