Skip to content

Commit 03e9649

Browse files
committed
Fix tainting of function calls absent taintable params
1 parent 56ef220 commit 03e9649

File tree

2 files changed

+57
-33
lines changed

2 files changed

+57
-33
lines changed

src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php

+33-32
Original file line numberDiff line numberDiff line change
@@ -1025,49 +1025,50 @@ private static function getFunctionCallReturnType(
10251025

10261026
if ($codebase->taint
10271027
&& $function_storage
1028-
&& $function_storage->return_source_params
10291028
&& $stmt_type
10301029
&& $codebase->config->trackTaintsInPath($statements_analyzer->getFilePath())
10311030
) {
1032-
foreach ($function_storage->return_source_params as $i) {
1033-
if (!isset($stmt->args[$i])) {
1034-
continue;
1035-
}
1031+
$return_location = new CodeLocation($statements_analyzer->getSource(), $stmt);
10361032

1037-
$arg_location = new CodeLocation(
1038-
$statements_analyzer->getSource(),
1039-
$stmt->args[$i]->value
1040-
);
1033+
$function_return_sink = TaintNode::getForMethodReturn(
1034+
$function_id,
1035+
$function_id,
1036+
$return_location,
1037+
$function_storage->specialize_call ? $return_location : null
1038+
);
10411039

1042-
$return_location = new CodeLocation($statements_analyzer->getSource(), $stmt);
1040+
$codebase->taint->addTaintNode($function_return_sink);
10431041

1044-
$function_param_sink = TaintNode::getForMethodArgument(
1045-
$function_id,
1046-
$function_id,
1047-
$i,
1048-
$arg_location,
1049-
$function_storage->specialize_call ? $return_location : null
1050-
);
1042+
$stmt_type->parent_nodes[] = $function_return_sink;
10511043

1052-
$codebase->taint->addTaintNode($function_param_sink);
1044+
if ($function_storage->return_source_params) {
1045+
foreach ($function_storage->return_source_params as $i) {
1046+
if (!isset($stmt->args[$i])) {
1047+
continue;
1048+
}
10531049

1054-
$function_return_sink = TaintNode::getForMethodReturn(
1055-
$function_id,
1056-
$function_id,
1057-
$return_location,
1058-
$function_storage->specialize_call ? $return_location : null
1059-
);
1050+
$arg_location = new CodeLocation(
1051+
$statements_analyzer->getSource(),
1052+
$stmt->args[$i]->value
1053+
);
10601054

1061-
$codebase->taint->addTaintNode($function_return_sink);
1055+
$function_param_sink = TaintNode::getForMethodArgument(
1056+
$function_id,
1057+
$function_id,
1058+
$i,
1059+
$arg_location,
1060+
$function_storage->specialize_call ? $return_location : null
1061+
);
10621062

1063-
$codebase->taint->addPath(
1064-
$function_param_sink,
1065-
$function_return_sink,
1066-
$function_storage->added_taints,
1067-
$function_storage->removed_taints
1068-
);
1063+
$codebase->taint->addTaintNode($function_param_sink);
10691064

1070-
$stmt_type->parent_nodes[] = $function_return_sink;
1065+
$codebase->taint->addPath(
1066+
$function_param_sink,
1067+
$function_return_sink,
1068+
$function_storage->added_taints,
1069+
$function_storage->removed_taints
1070+
);
1071+
}
10711072
}
10721073
}
10731074

tests/TaintTest.php

+24-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ class TaintTest extends TestCase
99
/**
1010
* @return void
1111
*/
12-
public function testTaintedInputFromReturnTypeSimple()
12+
public function testTaintedInputFromMethodReturnTypeSimple()
1313
{
1414
$this->expectException(\Psalm\Exception\CodeException::class);
1515
$this->expectExceptionMessage('TaintedInput');
@@ -38,6 +38,29 @@ public function deleteUser(PDO $pdo) : void {
3838
$this->analyzeFile('somefile.php', new Context());
3939
}
4040

41+
/**
42+
* @return void
43+
*/
44+
public function testTaintedInputFromFunctionReturnType()
45+
{
46+
$this->expectException(\Psalm\Exception\CodeException::class);
47+
$this->expectExceptionMessage('TaintedInput');
48+
49+
$this->project_analyzer->trackTaintedInputs();
50+
51+
$this->addFile(
52+
'somefile.php',
53+
'<?php
54+
function getName() : string {
55+
return $_GET["name"] ?? "unknown";
56+
}
57+
58+
echo getName();'
59+
);
60+
61+
$this->analyzeFile('somefile.php', new Context());
62+
}
63+
4164
/**
4265
* @return void
4366
*/

0 commit comments

Comments
 (0)