Skip to content

Commit

Permalink
Merge pull request #10242 from cgocast/5.x
Browse files Browse the repository at this point in the history 
Allow tainted numerics except for 'html' and 'has_quotes'
  • Loading branch information
@orklah
orklah authored Oct 16, 2023
2 parents f1fc9c4 + eea7c33 commit 7a7d6a2
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 57 deletions.
17 changes: 17 additions & 0 deletions src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
use Psalm\Type\Union;
use UnexpectedValueException;

use function array_merge;
use function in_array;
use function strlen;

Expand Down Expand Up @@ -170,6 +171,14 @@ public static function analyze(
$removed_taints = $codebase->config->eventDispatcher->dispatchRemoveTaints($event);

if ($stmt_left_type && $stmt_left_type->parent_nodes) {
// numeric types can't be tainted html or has_quotes, neither can bool
if ($statements_analyzer->data_flow_graph instanceof TaintFlowGraph
&& $stmt_left_type->isSingle()
&& ($stmt_left_type->isInt() || $stmt_left_type->isFloat() || $stmt_left_type->isBool())
) {
$removed_taints = array_merge($removed_taints, array('html', 'has_quotes'));
}

foreach ($stmt_left_type->parent_nodes as $parent_node) {
$statements_analyzer->data_flow_graph->addPath(
$parent_node,
Expand All @@ -182,6 +191,14 @@ public static function analyze(
}

if ($stmt_right_type && $stmt_right_type->parent_nodes) {
// numeric types can't be tainted html or has_quotes, neither can bool
if ($statements_analyzer->data_flow_graph instanceof TaintFlowGraph
&& $stmt_right_type->isSingle()
&& ($stmt_right_type->isInt() || $stmt_right_type->isFloat() || $stmt_right_type->isBool())
) {
$removed_taints = array_merge($removed_taints, array('html', 'has_quotes'));
}

foreach ($stmt_right_type->parent_nodes as $parent_node) {
$statements_analyzer->data_flow_graph->addPath(
$parent_node,
Expand Down
15 changes: 8 additions & 7 deletions src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
use Psalm\Type\Atomic\TNamedObject;
use Psalm\Type\Union;

use function array_merge;
use function count;
use function explode;
use function implode;
Expand Down Expand Up @@ -1490,19 +1491,19 @@ private static function processTaintedness(
return;
}

// numeric types can't be tainted, neither can bool
$event = new AddRemoveTaintsEvent($expr, $context, $statements_analyzer, $codebase);

$added_taints = $codebase->config->eventDispatcher->dispatchAddTaints($event);
$removed_taints = $codebase->config->eventDispatcher->dispatchRemoveTaints($event);

// numeric types can't be tainted html or has_quotes, neither can bool
if ($statements_analyzer->data_flow_graph instanceof TaintFlowGraph
&& $input_type->isSingle()
&& ($input_type->isInt() || $input_type->isFloat() || $input_type->isBool())
) {
return;
$removed_taints = array_merge($removed_taints, array('html', 'has_quotes'));
}

$event = new AddRemoveTaintsEvent($expr, $context, $statements_analyzer, $codebase);

$added_taints = $codebase->config->eventDispatcher->dispatchAddTaints($event);
$removed_taints = $codebase->config->eventDispatcher->dispatchRemoveTaints($event);

if ($function_param->type && $function_param->type->isString() && !$input_type->isString()) {
$input_type = CastAnalyzer::castStringAttempt(
$statements_analyzer,
Expand Down
23 changes: 5 additions & 18 deletions src/Psalm/Internal/Analyzer/Statements/Expression/CastAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,14 +142,9 @@ public static function analyze(
}
}

if ($statements_analyzer->data_flow_graph instanceof VariableUseGraph
) {
$type = new Union([new TBool()], [
'parent_nodes' => $maybe_type->parent_nodes ?? [],
]);
} else {
$type = Type::getBool();
}
$type = new Union([new TBool()], [
'parent_nodes' => $maybe_type->parent_nodes ?? [],
]);

$statements_analyzer->node_data->setType($stmt, $type);

Expand Down Expand Up @@ -328,11 +323,7 @@ public static function castIntAttempt(

$atomic_types = $stmt_type->getAtomicTypes();

$parent_nodes = [];

if ($statements_analyzer->data_flow_graph instanceof VariableUseGraph) {
$parent_nodes = $stmt_type->parent_nodes;
}
$parent_nodes = $stmt_type->parent_nodes;

while ($atomic_types) {
$atomic_type = array_pop($atomic_types);
Expand Down Expand Up @@ -518,11 +509,7 @@ public static function castFloatAttempt(

$atomic_types = $stmt_type->getAtomicTypes();

$parent_nodes = [];

if ($statements_analyzer->data_flow_graph instanceof VariableUseGraph) {
$parent_nodes = $stmt_type->parent_nodes;
}
$parent_nodes = $stmt_type->parent_nodes;

while ($atomic_types) {
$atomic_type = array_pop($atomic_types);
Expand Down
66 changes: 34 additions & 32 deletions tests/TaintTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,23 +177,6 @@ public function deleteUser(PDO $pdo, string $userId) : void {
}
}',
],
'untaintedInputAfterIntCast' => [
'code' => '<?php
class A {
public function getUserId() : int {
return (int) $_GET["user_id"];
}
public function getAppendedUserId() : string {
return "aaaa" . $this->getUserId();
}
public function deleteUser(PDO $pdo) : void {
$userId = $this->getAppendedUserId();
$pdo->exec("delete from users where user_id = " . $userId);
}
}',
],
'specializedCoreFunctionCall' => [
'code' => '<?php
$a = (string) ($data["user_id"] ?? "");
Expand Down Expand Up @@ -698,21 +681,6 @@ function query(string $sql) {}
$value = $_GET["value"];
$result = fetch($value);',
],
'NoTaintForIntTypeCastUsingAnnotatedSink' => [
'code' => '<?php // --taint-analysis
function fetch($id): string
{
return query("SELECT * FROM table WHERE id=" . (int)$id);
}
/**
* @return string
* @psalm-taint-sink sql $sql
* @psalm-taint-specialize
*/
function query(string $sql) {}
$value = $_GET["value"];
$result = fetch($value);',
],
'dontTaintArrayWithDifferentOffsetUpdated' => [
'code' => '<?php
function foo() {
Expand Down Expand Up @@ -884,6 +852,40 @@ public function deleteUser(PDO $pdo) : void {
}',
'error_message' => 'TaintedSql',
],
'taintedInputAfterIntCast' => [
'code' => '<?php
class A {
public function getUserId() : int {
return (int) $_GET["user_id"];
}
public function getAppendedUserId() : string {
return "aaaa" . $this->getUserId();
}
public function deleteUser(PDO $pdo) : void {
$userId = $this->getAppendedUserId();
$pdo->exec("delete from users where user_id = " . $userId);
}
}',
'error_message' => 'TaintedSql',
],
'TaintForIntTypeCastUsingAnnotatedSink' => [
'code' => '<?php // --taint-analysis
function fetch($id): string
{
return query("SELECT * FROM table WHERE id=" . (int)$id);
}
/**
* @return string
* @psalm-taint-sink sql $sql
* @psalm-taint-specialize
*/
function query(string $sql) {}
$value = $_GET["value"];
$result = fetch($value);',
'error_message' => 'TaintedSql',
],
'taintedInputFromReturnTypeWithBranch' => [
'code' => '<?php
class A {
Expand Down

0 comments on commit 7a7d6a2

Please sign in to comment.