Skip to content

Commit

Permalink
Improve tracking of array taints
Browse files Browse the repository at this point in the history
  • Loading branch information
@muglug
muglug committed Jun 18, 2020
1 parent 562a7c1 commit 49f0592
Show file tree
Hide file tree
Showing 18 changed files with 370 additions and 136 deletions.
25 changes: 21 additions & 4 deletions src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php
View file
Expand Up @@ -204,11 +204,28 @@ public static function analyze(

if ($codebase->taint) {
if ($item_value_type = $statements_analyzer->node_data->getType($item->value)) {
$parent_taint_nodes = array_merge($parent_taint_nodes, $item_value_type->parent_nodes ?: []);
}
if ($item_value_type->parent_nodes) {
$var_location = new CodeLocation($statements_analyzer->getSource(), $item);

$new_parent_node = \Psalm\Internal\Taint\TaintNode::getForAssignment(
'array'
. ($item_key_value !== null ? '[\'' . $item_key_value . '\']' : ''),
$var_location
);

$codebase->taint->addTaintNode($new_parent_node);

foreach ($item_value_type->parent_nodes as $parent_node) {
$codebase->taint->addPath(
$parent_node,
$new_parent_node,
'array-assignment'
. ($item_key_value !== null ? '-\'' . $item_key_value . '\'' : '')
);
}

if ($item->key && ($item_key_type = $statements_analyzer->node_data->getType($item->key))) {
$parent_taint_nodes = array_merge($parent_taint_nodes, $item_key_type->parent_nodes ?: []);
$parent_taint_nodes = array_merge($parent_taint_nodes, [$new_parent_node]);
}
}
}

Expand Down
92 changes: 71 additions & 21 deletions src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/ArrayAssignmentAnalyzer.php
View file
Expand Up @@ -150,15 +150,6 @@ public static function updateArrayType(

$child_stmt = null;

$parent_taint_nodes = [];

if ($codebase->taint
&& $assign_value
&& ($assign_value_type = $statements_analyzer->node_data->getType($assign_value))
) {
$parent_taint_nodes = $assign_value_type->parent_nodes;
}

// First go from the root element up, and go as far as we can to figure out what
// array types there are
while ($child_stmts) {
Expand All @@ -170,6 +161,8 @@ public static function updateArrayType(

$child_stmt_dim_type = null;

$dim_value = null;

if ($child_stmt->dim) {
if (ExpressionAnalyzer::analyze(
$statements_analyzer,
Expand All @@ -189,27 +182,28 @@ public static function updateArrayType(
&& $child_stmt_dim_type->isSingleStringLiteral())
) {
if ($child_stmt->dim instanceof PhpParser\Node\Scalar\String_) {
$value = $child_stmt->dim->value;
$dim_value = $child_stmt->dim->value;
} else {
$value = $child_stmt_dim_type->getSingleStringLiteral()->value;
$dim_value = $child_stmt_dim_type->getSingleStringLiteral()->value;
}

if (preg_match('/^(0|[1-9][0-9]*)$/', $value)) {
$var_id_additions[] = '[' . $value . ']';
if (preg_match('/^(0|[1-9][0-9]*)$/', $dim_value)) {
$var_id_additions[] = '[' . $dim_value . ']';
} else {
$var_id_additions[] = '[\'' . $dim_value . '\']';
}
$var_id_additions[] = '[\'' . $value . '\']';
} elseif ($child_stmt->dim instanceof PhpParser\Node\Scalar\LNumber
|| (($child_stmt->dim instanceof PhpParser\Node\Expr\ConstFetch
|| $child_stmt->dim instanceof PhpParser\Node\Expr\ClassConstFetch)
&& $child_stmt_dim_type->isSingleIntLiteral())
) {
if ($child_stmt->dim instanceof PhpParser\Node\Scalar\LNumber) {
$value = $child_stmt->dim->value;
$dim_value = $child_stmt->dim->value;
} else {
$value = $child_stmt_dim_type->getSingleIntLiteral()->value;
$dim_value = $child_stmt_dim_type->getSingleIntLiteral()->value;
}

$var_id_additions[] = '[' . $value . ']';
$var_id_additions[] = '[' . $dim_value . ']';
} elseif ($child_stmt->dim instanceof PhpParser\Node\Expr\Variable
&& is_string($child_stmt->dim->name)
) {
Expand Down Expand Up @@ -302,6 +296,15 @@ public static function updateArrayType(

$child_stmt_type = $assignment_type;
$statements_analyzer->node_data->setType($child_stmt, $assignment_type);

self::taintArrayAssignment(
$statements_analyzer,
$child_stmt->var,
$array_type,
$assignment_type,
$array_var_id,
$dim_value
);
}

$current_type = $child_stmt_type;
Expand Down Expand Up @@ -440,11 +443,24 @@ public static function updateArrayType(

array_pop($var_id_additions);

$array_var_id = null;

if ($root_var_id) {
$array_var_id = $root_var_id . implode('', $var_id_additions);
$context->vars_in_scope[$array_var_id] = clone $child_stmt_type;
$context->possibly_assigned_var_ids[$array_var_id] = true;
}

if ($codebase->taint) {
self::taintArrayAssignment(
$statements_analyzer,
$child_stmt->var,
$statements_analyzer->node_data->getType($child_stmt->var) ?: Type::getMixed(),
$new_child_type,
$array_var_id,
$key_value
);
}
}

$root_is_string = $root_type->isString();
Expand Down Expand Up @@ -670,10 +686,6 @@ public static function updateArrayType(
$root_type = $new_child_type;
}

if ($codebase->taint && $parent_taint_nodes) {
$root_type->parent_nodes = \array_merge($parent_taint_nodes, $root_type->parent_nodes ?: []);
}

$statements_analyzer->node_data->setType($root_array_expr, $root_type);

if ($root_array_expr instanceof PhpParser\Node\Expr\PropertyFetch) {
Expand Down Expand Up @@ -730,4 +742,42 @@ public static function updateArrayType(

return null;
}

/**
* @param int|string|null $item_key_value
*/
private static function taintArrayAssignment(
StatementsAnalyzer $statements_analyzer,
PhpParser\Node\Expr $stmt,
Type\Union $stmt_type,
Type\Union $child_stmt_type,
?string $array_var_id,
$item_key_value
) : void {
$codebase = $statements_analyzer->getCodebase();

if ($codebase->taint
&& $child_stmt_type->parent_nodes
) {
$var_location = new \Psalm\CodeLocation($statements_analyzer->getSource(), $stmt);

$new_parent_node = \Psalm\Internal\Taint\TaintNode::getForAssignment(
$array_var_id ?: 'array-assignment',
$var_location
);

$codebase->taint->addTaintNode($new_parent_node);

foreach ($child_stmt_type->parent_nodes as $parent_node) {
$codebase->taint->addPath(
$parent_node,
$new_parent_node,
'array-assignment'
. ($item_key_value !== null ? '-\'' . $item_key_value . '\'' : '')
);
}

$stmt_type->parent_nodes[] = $new_parent_node;
}
}
}
4 changes: 2 additions & 2 deletions src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/PropertyAssignmentAnalyzer.php
View file
Expand Up @@ -1098,11 +1098,11 @@ private static function taintProperty(

$codebase->taint->addTaintNode($property_node);

$codebase->taint->addPath($localized_property_node, $property_node);
$codebase->taint->addPath($localized_property_node, $property_node, 'property-assignment');

if ($assignment_value_type->parent_nodes) {
foreach ($assignment_value_type->parent_nodes as $parent_node) {
$codebase->taint->addPath($parent_node, $localized_property_node);
$codebase->taint->addPath($parent_node, $localized_property_node, '=');
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php
View file
Expand Up @@ -951,7 +951,7 @@ public static function analyze(
$codebase->taint->addTaintNode($new_parent_node);

foreach ($context->vars_in_scope[$var_id]->parent_nodes as $parent_node) {
$codebase->taint->addPath($parent_node, $new_parent_node, [], $removed_taints);
$codebase->taint->addPath($parent_node, $new_parent_node, '=', [], $removed_taints);
}

$context->vars_in_scope[$var_id]->parent_nodes = [$new_parent_node];
Expand Down Expand Up @@ -1133,13 +1133,13 @@ public static function analyzeAssignmentOperation(

if ($stmt_left_type && $stmt_left_type->parent_nodes) {
foreach ($stmt_left_type->parent_nodes as $parent_node) {
$codebase->taint->addPath($parent_node, $new_parent_node);
$codebase->taint->addPath($parent_node, $new_parent_node, 'concat');
}
}

if ($stmt_right_type && $stmt_right_type->parent_nodes) {
foreach ($stmt_right_type->parent_nodes as $parent_node) {
$codebase->taint->addPath($parent_node, $new_parent_node);
$codebase->taint->addPath($parent_node, $new_parent_node, 'concat');
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php
View file
Expand Up @@ -120,13 +120,13 @@ public static function analyze(

if ($stmt_left_type && $stmt_left_type->parent_nodes) {
foreach ($stmt_left_type->parent_nodes as $parent_node) {
$codebase->taint->addPath($parent_node, $new_parent_node);
$codebase->taint->addPath($parent_node, $new_parent_node, 'concat');
}
}

if ($stmt_right_type && $stmt_right_type->parent_nodes) {
foreach ($stmt_right_type->parent_nodes as $parent_node) {
$codebase->taint->addPath($parent_node, $new_parent_node);
$codebase->taint->addPath($parent_node, $new_parent_node, 'concat');
}
}
}
Expand Down

0 comments on commit 49f0592

Please sign in to comment.