danog/psalm
1
0
Fork 0
mirror of https://github.com/danog/psalm.git synced 2024-11-26 20:34:47 +01:00
Code Issues Projects Releases Wiki Activity

Fix #4599 - propagate taints to parent callers where necessary

Browse Source
This commit is contained in:
Matt Brown 2020-11-18 09:59:54 -05:00 committed by Daniil Gentili
parent 99d094b5e0
commit 3b8a76d520
Signed by: danog
GPG Key ID: 8C1BE3B34B230CA7
8 changed files with 180 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
src/Psalm/Internal/Analyzer/Statements/EchoAnalyzer.php
View File

@ -75,6 +75,7 @@ class EchoAnalyzer
Type::getString(),
null,
'echo',
null,
(int)$i,
new CodeLocation($statements_analyzer->getSource(), $expr),
$expr,

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

@ -15,6 +15,7 @@ use Psalm\Internal\Type\Comparator\UnionTypeComparator;
use Psalm\Internal\DataFlow\TaintSink;
use Psalm\Internal\DataFlow\DataFlowNode;
use Psalm\Internal\Codebase\TaintFlowGraph;
use Psalm\Internal\MethodIdentifier;
use Psalm\Internal\Type\TemplateResult;
use Psalm\Internal\Type\UnionTemplateHandler;
use Psalm\CodeLocation;
@ -55,6 +56,7 @@ class ArgumentAnalyzer
public static function checkArgumentMatches(
StatementsAnalyzer $statements_analyzer,
?string $cased_method_id,
?MethodIdentifier $method_id,
?string $self_fq_class_name,
?string $static_fq_class_name,
CodeLocation $function_call_location,
@ -165,6 +167,7 @@ class ArgumentAnalyzer
$statements_analyzer,
$codebase,
$cased_method_id,
$method_id,
$self_fq_class_name,
$static_fq_class_name,
$function_call_location,
@ -196,6 +199,7 @@ class ArgumentAnalyzer
StatementsAnalyzer $statements_analyzer,
Codebase $codebase,
?string $cased_method_id,
?MethodIdentifier $method_id,
?string $self_fq_class_name,
?string $static_fq_class_name,
CodeLocation $function_call_location,
@ -397,6 +401,7 @@ class ArgumentAnalyzer
self::processTaintedness(
$statements_analyzer,
$cased_method_id,
$method_id,
$argument_offset,
$arg_location,
$function_call_location,
@ -467,6 +472,7 @@ class ArgumentAnalyzer
$fleshed_out_type,
$fleshed_out_signature_type,
$cased_method_id,
$method_id,
$argument_offset,
new CodeLocation($statements_analyzer->getSource(), $arg->value),
$arg->value,
@ -494,6 +500,7 @@ class ArgumentAnalyzer
Type\Union $param_type,
?Type\Union $signature_param_type,
?string $cased_method_id,
?MethodIdentifier $method_id,
int $argument_offset,
CodeLocation $arg_location,
PhpParser\Node\Expr $input_expr,
@ -512,13 +519,9 @@ class ArgumentAnalyzer
&& !$input_type->hasMixed()
&& !$param_type->from_docblock
&& !$param_type->had_template
&& $cased_method_id
&& strpos($cased_method_id, '::')
&& !strpos($cased_method_id, '__')
&& $method_id
&& strpos($method_id->method_name, '__') !== 0
) {
$method_parts = explode('::', $cased_method_id);
$method_id = new \Psalm\Internal\MethodIdentifier($method_parts[0], strtolower($method_parts[1]));
$declaring_method_id = $codebase->methods->getDeclaringMethodId($method_id);
if ($declaring_method_id) {
@ -541,6 +544,7 @@ class ArgumentAnalyzer
self::processTaintedness(
$statements_analyzer,
$cased_method_id,
$method_id,
$argument_offset,
$arg_location,
$function_call_location,
@ -606,6 +610,7 @@ class ArgumentAnalyzer
$input_type = self::processTaintedness(
$statements_analyzer,
$cased_method_id,
$method_id,
$argument_offset,
$arg_location,
$function_call_location,
@ -694,6 +699,7 @@ class ArgumentAnalyzer
$input_type = self::processTaintedness(
$statements_analyzer,
$cased_method_id,
$method_id,
$argument_offset,
$arg_location,
$function_call_location,
@ -1227,6 +1233,7 @@ class ArgumentAnalyzer
private static function processTaintedness(
StatementsAnalyzer $statements_analyzer,
string $cased_method_id,
?MethodIdentifier $method_id,
int $argument_offset,
CodeLocation $arg_location,
CodeLocation $function_call_location,
@ -1279,11 +1286,13 @@ class ArgumentAnalyzer
);
if ($statements_analyzer->data_flow_graph instanceof TaintFlowGraph
&& strpos($cased_method_id, '::')
&& !strpos($cased_method_id, '::__construct')
&& $method_id
&& $method_id->method_name !== '__construct'
) {
[$fq_classlike_name, $cased_method_name] = explode('::', $cased_method_id);
$method_name = strtolower($cased_method_name);
$fq_classlike_name = $method_id->fq_class_name;
$method_name = $method_id->method_name;
$cased_method_name = explode('::', $cased_method_id)[1];
$class_storage = $codebase->classlike_storage_provider->get($fq_classlike_name);
foreach ($class_storage->dependent_classlikes as $dependent_classlike_lc => $_) {
@ -1301,21 +1310,23 @@ class ArgumentAnalyzer
$statements_analyzer->data_flow_graph->addNode($new_sink);
$statements_analyzer->data_flow_graph->addPath($method_node, $new_sink, 'arg');
}
}
}
if (isset($class_storage->overridden_method_ids[$method_name])) {
foreach ($class_storage->overridden_method_ids[$method_name] as $parent_method_id) {
$new_sink = DataFlowNode::getForMethodArgument(
(string) $parent_method_id,
$codebase->methods->getCasedMethodId($parent_method_id),
$argument_offset,
$arg_location,
null
);
if ($method_id && $statements_analyzer->data_flow_graph instanceof TaintFlowGraph) {
$declaring_method_id = $codebase->methods->getDeclaringMethodId($method_id);
$statements_analyzer->data_flow_graph->addNode($new_sink);
$statements_analyzer->data_flow_graph->addPath($method_node, $new_sink, 'arg');
}
}
if ($declaring_method_id && (string) $declaring_method_id !== (string) $method_id) {
$new_sink = DataFlowNode::getForMethodArgument(
(string) $declaring_method_id,
$codebase->methods->getCasedMethodId($declaring_method_id),
$argument_offset,
$arg_location,
null
);
$statements_analyzer->data_flow_graph->addNode($new_sink);
$statements_analyzer->data_flow_graph->addPath($method_node, $new_sink, 'arg');
}
}

8
src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentsAnalyzer.php
View File

@ -446,7 +446,7 @@ class ArgumentsAnalyzer
$codebase = $statements_analyzer->getCodebase();
if ($method_id) {
if (!$in_call_map && $method_id instanceof \Psalm\Internal\MethodIdentifier) {
if (!$in_call_map && $method_id instanceof MethodIdentifier) {
$fq_class_name = $method_id->fq_class_name;
}
@ -463,7 +463,7 @@ class ArgumentsAnalyzer
}
}
if ($method_id instanceof \Psalm\Internal\MethodIdentifier) {
if ($method_id instanceof MethodIdentifier) {
$cased_method_id = $codebase->methods->getCasedMethodId($method_id);
} elseif ($function_storage) {
$cased_method_id = $function_storage->cased_name;
@ -474,7 +474,7 @@ class ArgumentsAnalyzer
$static_fq_class_name = $fq_class_name;
$self_fq_class_name = $fq_class_name;
if ($method_id instanceof \Psalm\Internal\MethodIdentifier) {
if ($method_id instanceof MethodIdentifier) {
$declaring_method_id = $codebase->methods->getDeclaringMethodId($method_id);
if ($declaring_method_id && (string)$declaring_method_id !== (string)$method_id) {
@ -627,6 +627,7 @@ class ArgumentsAnalyzer
ArgumentAnalyzer::checkArgumentMatches(
$statements_analyzer,
$cased_method_id,
$method_id instanceof MethodIdentifier ? $method_id : null,
$self_fq_class_name,
$static_fq_class_name,
$code_location,
@ -712,6 +713,7 @@ class ArgumentsAnalyzer
if (ArgumentAnalyzer::checkArgumentMatches(
$statements_analyzer,
$cased_method_id,
$method_id instanceof MethodIdentifier ? $method_id : null,
$self_fq_class_name,
$static_fq_class_name,
$code_location,

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

@ -237,13 +237,33 @@ class MethodCallReturnTypeFetcher
$node_location = new CodeLocation($statements_analyzer, $name_expr);
$is_declaring = (string) $declaring_method_id === (string) $method_id;
$method_call_node = DataFlowNode::getForMethodReturn(
(string) $method_id,
$cased_method_id,
$method_storage->signature_return_type_location ?: $method_storage->location,
$is_declaring ? ($method_storage->signature_return_type_location ?: $method_storage->location) : null,
$method_storage->specialize_call ? $node_location : null
);
if (!$is_declaring) {
$cased_declaring_method_id = $codebase->methods->getCasedMethodId($declaring_method_id);
$declaring_method_call_node = DataFlowNode::getForMethodReturn(
(string) $declaring_method_id,
$cased_declaring_method_id,
$method_storage->signature_return_type_location ?: $method_storage->location,
$method_storage->specialize_call ? $node_location : null
);
$statements_analyzer->data_flow_graph->addNode($declaring_method_call_node);
$statements_analyzer->data_flow_graph->addPath(
$declaring_method_call_node,
$method_call_node,
'parent'
);
}
$statements_analyzer->data_flow_graph->addNode($method_call_node);
$return_type_candidate->parent_nodes = [

1
src/Psalm/Internal/Analyzer/Statements/Expression/ExitAnalyzer.php
View File

@ -60,6 +60,7 @@ class ExitAnalyzer
new Type\Union([new TInt(), new TString()]),
null,
'exit',
null,
0,
new CodeLocation($statements_analyzer->getSource(), $stmt->expr),
$stmt->expr,

1
src/Psalm/Internal/Analyzer/Statements/Expression/PrintAnalyzer.php
View File

@ -53,6 +53,7 @@ class PrintAnalyzer
Type::getString(),
null,
'print',
null,
0,
new CodeLocation($statements_analyzer->getSource(), $stmt->expr),
$stmt->expr,

2
src/Psalm/Internal/Codebase/TaintFlowGraph.php
View File

@ -364,7 +364,7 @@ class TaintFlowGraph extends DataFlowGraph
$path
);
break;
default:
$issue = new TaintedCustom(
'Detected tainted ' . $matching_taint,

116
tests/TaintTest.php
View File

@ -495,6 +495,27 @@ class TaintTest extends TestCase
new A($_GET["foo"]);'
],
'dontTaintThroughChildConstructorWhenMethodOverridden' => [
'<?php //--taint-analysis
class A {
private $taint;
public function __construct($taint) {
$this->taint = $taint;
}
public function getTaint() : string {
return $this->taint;
}
}
class B extends A {
public function __construct($taint) {}
}
$b = new B($_GET["bar"]);
echo $b->getTaint();'
],
];
}
@ -1651,6 +1672,101 @@ class TaintTest extends TestCase
curl_setopt($ch, CURLOPT_URL, $_GET[\'url\']);',
'error_message' => 'TaintedSSRF',
],
'taintThroughChildConstructorWithoutMethodOverride' => [
'<?php //--taint-analysis
class A {
private $taint;
public function __construct($taint) {
$this->taint = $taint;
}
public function getTaint() : string {
return $this->taint;
}
}
class B extends A {}
$b = new B($_GET["bar"]);
echo $b->getTaint();',
'error_message' => 'TaintedHtml',
],
'taintThroughChildConstructorCallingParentMethod' => [
'<?php //--taint-analysis
class A {
private $taint;
public function __construct($taint) {
$this->taint = $taint;
}
public function getTaint() : string {
return $this->taint;
}
}
class B extends A {}
class C extends B {}
$c = new C($_GET["bar"]);
function foo(B $b) {
echo $b->getTaint();
}',
'error_message' => 'TaintedHtml',
],
'taintThroughChildConstructorCallingGrandParentMethod' => [
'<?php //--taint-analysis
class A {
private $taint;
public function __construct($taint) {
$this->taint = $taint;
}
public function getTaint() : string {
return $this->taint;
}
}
class B extends A {}
class C extends B {}
$c = new C($_GET["bar"]);
function foo(A $a) {
echo $a->getTaint();
}',
'error_message' => 'TaintedHtml',
],
'taintThroughChildConstructorWhenMethodOverriddenWithParentConstructorCall' => [
'<?php //--taint-analysis
class A {
private $taint;
public function __construct($taint) {
$this->taint = $taint;
}
public function getTaint() : string {
return $this->taint;
}
}
class B extends A {
public function __construct($taint) {
parent::__construct($taint);
}
}
$b = new B($_GET["bar"]);
echo $b->getTaint();',
'error_message' => 'TaintedHtml',
],
/*
// TODO: Stubs do not support this type of inference even with $this->message = $message.
// Most uses of getMessage() would be with caught exceptions, so this is not representative of real code.