Merge pull request #6993 from orklah/taint-numerics

Taint can't be transmitted through numerics nor bool
vimeo · Nov 25, 2021 · 2fbad1b · 2fbad1b
2 parents 55b2b6b + ab61eae
commit 2fbad1b
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 30 deletions.
diff --git a/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php b/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php
@@ -12,6 +12,7 @@
 use Psalm\Internal\Analyzer\FunctionLike\ReturnTypeAnalyzer;
 use Psalm\Internal\Analyzer\FunctionLike\ReturnTypeCollector;
 use Psalm\Internal\Analyzer\Statements\ExpressionAnalyzer;
+use Psalm\Internal\Codebase\TaintFlowGraph;
 use Psalm\Internal\DataFlow\DataFlowNode;
 use Psalm\Internal\FileManipulation\FunctionDocblockManipulator;
 use Psalm\Internal\Type\Comparator\TypeComparisonResult;
@@ -1032,40 +1033,49 @@ private function processParams(
             if ($statements_analyzer->data_flow_graph
                 && $function_param->location
             ) {
-                $param_assignment = DataFlowNode::getForAssignment(
-                    '$' . $function_param->name,
-                    $function_param->location
-                );
+                //don't add to taint flow graph if the type can't transmit taints
+                if (!$statements_analyzer->data_flow_graph instanceof TaintFlowGraph
+                    || $function_param->type === null
+                    || !$function_param->type->isSingle()
+                    || (!$function_param->type->isInt()
+                        && !$function_param->type->isFloat()
+                        && !$function_param->type->isBool())
+                ) {
+                    $param_assignment = DataFlowNode::getForAssignment(
+                        '$' . $function_param->name,
+                        $function_param->location
+                    );
 
-                $statements_analyzer->data_flow_graph->addNode($param_assignment);
+                    $statements_analyzer->data_flow_graph->addNode($param_assignment);
 
-                if ($cased_method_id) {
-                    $type_source = DataFlowNode::getForMethodArgument(
-                        $cased_method_id,
-                        $cased_method_id,
-                        $offset,
-                        $function_param->location,
-                        null
-                    );
+                    if ($cased_method_id) {
+                        $type_source = DataFlowNode::getForMethodArgument(
+                            $cased_method_id,
+                            $cased_method_id,
+                            $offset,
+                            $function_param->location,
+                            null
+                        );
 
-                    $statements_analyzer->data_flow_graph->addPath($type_source, $param_assignment, 'param');
-                }
+                        $statements_analyzer->data_flow_graph->addPath($type_source, $param_assignment, 'param');
+                    }
 
-                if ($function_param->by_ref
-                    && $codebase->find_unused_variables
-                ) {
-                    $statements_analyzer->data_flow_graph->addPath(
-                        $param_assignment,
-                        new DataFlowNode('variable-use', 'variable use', null),
-                        'variable-use'
-                    );
-                }
+                    if ($function_param->by_ref
+                        && $codebase->find_unused_variables
+                    ) {
+                        $statements_analyzer->data_flow_graph->addPath(
+                            $param_assignment,
+                            new DataFlowNode('variable-use', 'variable use', null),
+                            'variable-use'
+                        );
+                    }
 
-                if ($storage->variadic) {
-                    $this->param_nodes += [$param_assignment->id => $param_assignment];
-                }
+                    if ($storage->variadic) {
+                        $this->param_nodes += [$param_assignment->id => $param_assignment];
+                    }
 
-                $var_type->parent_nodes += [$param_assignment->id => $param_assignment];
+                    $var_type->parent_nodes += [$param_assignment->id => $param_assignment];
+                }
             }
 
             $context->vars_in_scope['$' . $function_param->name] = $var_type;

diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php
@@ -1505,10 +1505,10 @@ private static function processTaintedness(
             return $input_type;
         }
 
-        // numeric types can't be tainted
+        // 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->isInt() || $input_type->isFloat() || $input_type->isBool())
         ) {
             return $input_type;
         }

diff --git a/tests/TaintTest.php b/tests/TaintTest.php
@@ -671,6 +671,36 @@ function takesArray(array $arr): void {
                     $var = $input + 1;
                     var_dump($var);'
             ],
+            'NoTaintForIntTypeHintUsingAnnotatedSink' => [
+                '<?php // --taint-analysis
+                    function fetch(int $id): string
+                    {
+                        return query("SELECT * FROM table WHERE id=" . $id);
+                    }
+                    /**
+                     * @return string
+                     * @psalm-taint-sink sql $sql
+                     * @psalm-taint-specialize
+                     */
+                    function query(string $sql) {}
+                    $value = $_GET["value"];
+                    $result = fetch($value);'
+            ],
+            'NoTaintForIntTypeCastUsingAnnotatedSink' => [
+                '<?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);'
+            ],
         ];
     }