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

Allow immutable classes to be specialised through calls

Browse Source
This commit is contained in:
Matt Brown 2020-11-19 01:38:20 -05:00 committed by Daniil Gentili
parent 106ab936f9
commit 005f394d8e
Signed by: danog
GPG Key ID: 8C1BE3B34B230CA7
6 changed files with 287 additions and 99 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php
View File

@ -242,6 +242,16 @@ abstract class FunctionLikeAnalyzer extends SourceAnalyzer
$context->vars_in_scope['$this'] = new Type\Union([$this_object_type]);
if ($codebase->taint_flow_graph
&& $storage->specialize_call
&& $storage->location
) {
$new_parent_node = DataFlowNode::getForAssignment('$this in ' . $method_id, $storage->location);
$codebase->taint_flow_graph->addNode($new_parent_node);
$context->vars_in_scope['$this']->parent_nodes += [$new_parent_node->id => $new_parent_node];
}
if ($storage->external_mutation_free
&& !$storage->mutation_free_inferred
) {

104
src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php
View File

@ -901,23 +901,13 @@ class AssignmentAnalyzer
) {
$context->vars_in_scope[$list_var_id]->parent_nodes = [];
} else {
$new_parent_node = DataFlowNode::getForAssignment($list_var_id, $var_location);
$statements_analyzer->data_flow_graph->addNode($new_parent_node);
foreach ($context->vars_in_scope[$list_var_id]->parent_nodes as $parent_node) {
$data_flow_graph->addPath(
$parent_node,
$new_parent_node,
'=',
[],
$removed_taints
);
}
$context->vars_in_scope[$list_var_id]->parent_nodes = [
$new_parent_node->id => $new_parent_node
];
self::taintAssignment(
$context->vars_in_scope[$list_var_id],
$data_flow_graph,
$list_var_id,
$var_location,
$removed_taints
);
}
}
}
@ -1101,17 +1091,13 @@ class AssignmentAnalyzer
} else {
$var_location = new CodeLocation($statements_analyzer->getSource(), $assign_var);
$new_parent_node = DataFlowNode::getForAssignment($var_id, $var_location);
$data_flow_graph->addNode($new_parent_node);
foreach ($context->vars_in_scope[$var_id]->parent_nodes as $parent_node) {
$data_flow_graph->addPath($parent_node, $new_parent_node, '=', [], $removed_taints);
}
$context->vars_in_scope[$var_id]->parent_nodes = [
$new_parent_node->id => $new_parent_node
];
self::taintAssignment(
$context->vars_in_scope[$var_id],
$data_flow_graph,
$var_id,
$var_location,
$removed_taints
);
}
}
}
@ -1234,6 +1220,68 @@ class AssignmentAnalyzer
}
}
/**
* @param array<string> $removed_taints
*/
private static function taintAssignment(
Type\Union $type,
\Psalm\Internal\Codebase\DataFlowGraph $data_flow_graph,
string $var_id,
CodeLocation $var_location,
array $removed_taints
) : void {
$parent_nodes = $type->parent_nodes;
$unspecialized_parent_nodes = \array_filter(
$parent_nodes,
function ($parent_node) {
return !$parent_node->specialization_key;
}
);
$specialized_parent_nodes = \array_filter(
$parent_nodes,
function ($parent_node) {
return (bool) $parent_node->specialization_key;
}
);
$new_parent_nodes = [];
foreach ($specialized_parent_nodes as $parent_node) {
$new_parent_node = DataFlowNode::getForAssignment($var_id, $var_location);
$new_parent_node->specialization_key = $parent_node->specialization_key;
$data_flow_graph->addNode($new_parent_node);
$new_parent_nodes += [$new_parent_node->id => $new_parent_node];
$data_flow_graph->addPath(
$parent_node,
$new_parent_node,
'=',
[],
$removed_taints
);
}
if ($unspecialized_parent_nodes) {
$new_parent_node = DataFlowNode::getForAssignment($var_id, $var_location);
$data_flow_graph->addNode($new_parent_node);
$new_parent_nodes += [$new_parent_node->id => $new_parent_node];
foreach ($unspecialized_parent_nodes as $parent_node) {
$data_flow_graph->addPath(
$parent_node,
$new_parent_node,
'=',
[],
$removed_taints
);
}
}
$type->parent_nodes = $new_parent_nodes;
}
public static function analyzeAssignmentOperation(
StatementsAnalyzer $statements_analyzer,
PhpParser\Node\Expr\AssignOp $stmt,

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

@ -239,37 +239,6 @@ class MethodCallReturnTypeFetcher
$is_declaring = (string) $declaring_method_id === (string) $method_id;
$method_call_node = DataFlowNode::getForMethodReturn(
(string) $method_id,
$cased_method_id,
$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 = [
$method_call_node->id => $method_call_node
];
if ($method_storage->specialize_call) {
$var_id = ExpressionIdentifier::getArrayVarId(
$var_expr,
@ -278,31 +247,155 @@ class MethodCallReturnTypeFetcher
);
if ($var_id && isset($context->vars_in_scope[$var_id])) {
$var_nodes = [];
$parent_nodes = $context->vars_in_scope[$var_id]->parent_nodes;
$unspecialized_parent_nodes = \array_filter(
$parent_nodes,
function ($parent_node) {
return !$parent_node->specialization_key;
}
);
$specialized_parent_nodes = \array_filter(
$parent_nodes,
function ($parent_node) {
return (bool) $parent_node->specialization_key;
}
);
$var_node = DataFlowNode::getForAssignment(
$var_id,
new CodeLocation($statements_analyzer, $var_expr)
);
$statements_analyzer->data_flow_graph->addNode($var_node);
if ($method_storage->location) {
$this_parent_node = DataFlowNode::getForAssignment(
'$this in ' . $method_id,
$method_storage->location
);
$statements_analyzer->data_flow_graph->addPath(
$method_call_node,
$var_node,
'method-call-' . $method_id->method_name
);
$stmt_var_type = clone $context->vars_in_scope[$var_id];
if ($context->vars_in_scope[$var_id]->parent_nodes) {
foreach ($context->vars_in_scope[$var_id]->parent_nodes as $parent_node) {
$statements_analyzer->data_flow_graph->addPath($parent_node, $var_node, '=');
foreach ($parent_nodes as $parent_node) {
$statements_analyzer->data_flow_graph->addPath($parent_node, $this_parent_node, '=');
}
}
$stmt_var_type->parent_nodes = [$var_node->id => $var_node];
$var_nodes[$var_node->id] = $var_node;
$method_call_nodes = [];
if ($unspecialized_parent_nodes) {
$method_call_node = DataFlowNode::getForMethodReturn(
(string) $method_id,
$cased_method_id,
$is_declaring ? ($method_storage->signature_return_type_location
?: $method_storage->location) : null,
$node_location
);
$method_call_nodes[$method_call_node->id] = $method_call_node;
}
foreach ($specialized_parent_nodes as $parent_node) {
$universal_method_call_node = DataFlowNode::getForMethodReturn(
(string) $method_id,
$cased_method_id,
$is_declaring ? ($method_storage->signature_return_type_location
?: $method_storage->location) : null,
null
);
$method_call_node = new DataFlowNode(
strtolower((string) $method_id),
$cased_method_id,
$is_declaring ? ($method_storage->signature_return_type_location
?: $method_storage->location) : null,
$parent_node->specialization_key
);
$statements_analyzer->data_flow_graph->addPath(
$universal_method_call_node,
$method_call_node,
'='
);
$method_call_nodes[$method_call_node->id] = $method_call_node;
}
foreach ($method_call_nodes as $method_call_node) {
$statements_analyzer->data_flow_graph->addNode($method_call_node);
foreach ($var_nodes as $var_node) {
$statements_analyzer->data_flow_graph->addNode($var_node);
$statements_analyzer->data_flow_graph->addPath(
$method_call_node,
$var_node,
'method-call-' . $method_id->method_name
);
}
if (!$is_declaring) {
$cased_declaring_method_id = $codebase->methods->getCasedMethodId($declaring_method_id);
$declaring_method_call_node = new DataFlowNode(
strtolower((string) $declaring_method_id),
$cased_declaring_method_id,
$method_storage->signature_return_type_location ?: $method_storage->location,
$method_call_node->specialization_key
);
$statements_analyzer->data_flow_graph->addNode($declaring_method_call_node);
$statements_analyzer->data_flow_graph->addPath(
$declaring_method_call_node,
$method_call_node,
'parent'
);
}
}
$return_type_candidate->parent_nodes = $method_call_nodes;
$stmt_var_type = clone $context->vars_in_scope[$var_id];
$stmt_var_type->parent_nodes = $var_nodes;
$context->vars_in_scope[$var_id] = $stmt_var_type;
}
} else {
$method_call_node = DataFlowNode::getForMethodReturn(
(string) $method_id,
$cased_method_id,
$is_declaring
? ($method_storage->signature_return_type_location ?: $method_storage->location)
: null,
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,
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 = [
$method_call_node->id => $method_call_node
];
}
if ($method_storage->taint_source_types) {

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

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

5
src/Psalm/Internal/DataFlow/DataFlowNode.php
View File

@ -95,14 +95,15 @@ class DataFlowNode
*/
final public static function getForAssignment(
string $var_id,
CodeLocation $assignment_location
CodeLocation $assignment_location,
?string $specialization_key = null
): self {
$id = $var_id
. '-' . $assignment_location->file_name
. ':' . $assignment_location->raw_file_start
. '-' . $assignment_location->raw_file_end;
return new static($id, $var_id, $assignment_location, null);
return new static($id, $var_id, $assignment_location, $specialization_key);
}
/**

84
tests/TaintTest.php
View File

@ -383,27 +383,27 @@ class TaintTest extends TestCase
UserUpdater::doDelete(new PDO(), $userObj);'
],
'taintPropertyWithoutPassingObject' => [
'<?phps
/** @psalm-immutable */
'<?php
/** @psalm-taint-specialize */
class User {
public string $id;
public function __construct(string $userId) {
$this->id = $userId;
}
}
class UserUpdater {
public static function doDelete(PDO $pdo, User $user) : void {
self::deleteUser($pdo, $user->id);
}
public static function deleteUser(PDO $pdo, string $userId) : void {
$pdo->exec("delete from users where user_id = " . $userId);
public function setId(string $userId) : void {
$this->id = $userId;
}
}
$userObj = new User((string) $_GET["user_id"]);',
function echoId(User $u2) : void {
echo $u2->id;
}
$u = new User("5");
echoId($u);
$u->setId($_GET["user_id"]);',
],
'specializeStaticMethod' => [
'<?php
@ -523,6 +523,27 @@ class TaintTest extends TestCase
$b = new B($_GET["bar"]);
echo $b->getTaint();'
],
'immutableClassTrackInputThroughMethod' => [
'<?php
/**
* @psalm-immutable
*/
class A {
private string $taint = "";
public function __construct(string $taint) {
$this->taint = $taint;
}
public function getTaint() : string {
return $this->taint;
}
}
$b = new A($_GET["bar"]);
$a = new A("bar");
echo $a->getTaint();',
],
];
}
@ -1236,20 +1257,14 @@ class TaintTest extends TestCase
}
}
class UserUpdater {
public static function doDelete(PDO $pdo, User $user) : void {
self::deleteUser($pdo, $user->id);
}
public static function deleteUser(PDO $pdo, string $userId) : void {
$pdo->exec("delete from users where user_id = " . $userId);
}
function echoId(User $u2) : void {
echo $u2->id;
}
$userObj = new User("5");
$userObj->setId((string) $_GET["user_id"]);
UserUpdater::doDelete(new PDO(), $userObj);',
'error_message' => 'TaintedSql',
$u = new User("5");
$u->setId($_GET["user_id"]);
echoId($u);',
'error_message' => 'TaintedHtml',
],
'ImplodeExplode' => [
'<?php
@ -1524,7 +1539,7 @@ class TaintTest extends TestCase
echo $a->isUnsafe();',
'error_message' => 'TaintedHtml',
],
'taintSpecializedInstanceProperty' => [
'doTaintSpecializedInstanceProperty' => [
'<?php
/** @psalm-taint-specialize */
class StringHolder {
@ -1802,6 +1817,27 @@ class TaintTest extends TestCase
echo $b->getTaint();',
'error_message' => 'TaintedHtml',
],
'immutableClassTrackInputThroughMethod' => [
'<?php
/**
* @psalm-immutable
*/
class A {
private string $taint = "";
public function __construct(string $taint) {
$this->taint = $taint;
}
public function getTaint() : string {
return $this->taint;
}
}
$a = new A($_GET["bar"]);
echo $a->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.