Skip to content

Commit

Permalink
Merge pull request #10729 from ohader/revert-10242-5.x
Browse files Browse the repository at this point in the history
  • Loading branch information
@weirdan
weirdan committed Feb 21, 2024
2 parents 2b64280 + 679aaf0 commit 4a5f433
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 64 deletions.
17 changes: 0 additions & 17 deletions src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
use Psalm\Type\Union;
use UnexpectedValueException;

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

Expand Down Expand Up @@ -171,14 +170,6 @@ 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 @@ -191,14 +182,6 @@ 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: 7 additions & 8 deletions src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@
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 @@ -1529,19 +1528,19 @@ private static function processTaintedness(
return;
}

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

$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: 18 additions & 5 deletions src/Psalm/Internal/Analyzer/Statements/Expression/CastAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,14 @@ public static function analyze(
}
}

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

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

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

$atomic_types = $stmt_type->getAtomicTypes();

$parent_nodes = $stmt_type->parent_nodes;
$parent_nodes = [];

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

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

$atomic_types = $stmt_type->getAtomicTypes();

$parent_nodes = $stmt_type->parent_nodes;
$parent_nodes = [];

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

while ($atomic_types) {
$atomic_type = array_pop($atomic_types);
Expand Down
66 changes: 32 additions & 34 deletions tests/TaintTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,23 @@ 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 @@ -681,6 +698,21 @@ 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 @@ -852,40 +884,6 @@ 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 4a5f433

Please sign in to comment.