Support taint flows in more functions

vimeo · Jun 22, 2020 · e8be2c5 · e8be2c5
1 parent 7f05b3c
commit e8be2c5
Show file tree

Hide file tree

Showing 7 changed files with 84 additions and 21 deletions.
diff --git a/src/Psalm/Internal/Analyzer/CommentAnalyzer.php b/src/Psalm/Internal/Analyzer/CommentAnalyzer.php
@@ -468,8 +468,9 @@ public static function extractFunctionDocblockInfo(PhpParser\Comment\Doc $commen
         }
 
         if (isset($parsed_docblock->tags['psalm-flow'])) {
-            $flow = trim(reset($parsed_docblock->tags['psalm-flow']));
-            $info->flow = $flow;
+            foreach ($parsed_docblock->tags['psalm-flow'] as $param) {
+                $info->flows[] = trim($param);
+            }
         }
 
         if (isset($parsed_docblock->tags['psalm-taint-sink'])) {

diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php
@@ -1092,7 +1092,7 @@ private static function taintReturnType(
                 }
             }
 
-            foreach ($function_storage->return_source_params as $i) {
+            foreach ($function_storage->return_source_params as $i => $path_type) {
                 if (!isset($stmt->args[$i])) {
                     continue;
                 }
@@ -1115,7 +1115,7 @@ private static function taintReturnType(
                 $codebase->taint->addPath(
                     $function_param_sink,
                     $function_return_sink,
-                    'arg',
+                    $path_type,
                     $function_storage->added_taints,
                     $removed_taints
                 );

diff --git a/src/Psalm/Internal/PhpVisitor/ReflectorVisitor.php b/src/Psalm/Internal/PhpVisitor/ReflectorVisitor.php
@@ -2515,21 +2515,35 @@ function (FunctionLikeParameter $p) {
         $storage->added_taints = $docblock_info->added_taints;
         $storage->removed_taints = $docblock_info->removed_taints;
 
-        if ($docblock_info->flow) {
-            $flow_parts = explode('->', $docblock_info->flow);
+        if ($docblock_info->flows) {
+            foreach ($docblock_info->flows as $flow) {
+                $path_type = 'arg';
 
-            if (isset($flow_parts[1]) && trim($flow_parts[1]) === 'return') {
-                $source_param_string = trim($flow_parts[0]);
+                $fancy_path_regex = '/-\(([a-z\-]+)\)->/';
 
-                if ($source_param_string[0] === '(' && substr($source_param_string, -1) === ')') {
-                    $source_params = preg_split('/, ?/', substr($source_param_string, 1, -1));
+                if (preg_match($fancy_path_regex, $flow, $matches)) {
+                    if (isset($matches[1])) {
+                        $path_type = $matches[1];
+                    }
+
+                    $flow = preg_replace($fancy_path_regex, '->', $flow);
+                }
+
+                $flow_parts = explode('->', $flow);
 
-                    foreach ($source_params as $source_param) {
-                        $source_param = substr($source_param, 1);
+                if (isset($flow_parts[1]) && trim($flow_parts[1]) === 'return') {
+                    $source_param_string = trim($flow_parts[0]);
 
-                        foreach ($storage->params as $i => $param_storage) {
-                            if ($param_storage->name === $source_param) {
-                                $storage->return_source_params[] = $i;
+                    if ($source_param_string[0] === '(' && substr($source_param_string, -1) === ')') {
+                        $source_params = preg_split('/, ?/', substr($source_param_string, 1, -1));
+
+                        foreach ($source_params as $source_param) {
+                            $source_param = substr($source_param, 1);
+
+                            foreach ($storage->params as $i => $param_storage) {
+                                if ($param_storage->name === $source_param) {
+                                    $storage->return_source_params[$i] = $path_type;
+                                }
                             }
                         }
                     }

diff --git a/src/Psalm/Internal/Scanner/FunctionDocblockComment.php b/src/Psalm/Internal/Scanner/FunctionDocblockComment.php
@@ -91,9 +91,9 @@ class FunctionDocblockComment
     /**
      * Represents the flow from function params to return type
      *
-     * @var ?string
+     * @var array<string>
      */
-    public $flow;
+    public $flows = [];
 
     /**
      * @var array<string>

diff --git a/src/Psalm/Internal/Stubs/CoreGenericFunctions.phpstub b/src/Psalm/Internal/Stubs/CoreGenericFunctions.phpstub
@@ -480,6 +480,27 @@ function strtolower(string $str) : string {}
  */
 function strtoupper(string $str) : string {}
 
+/**
+ * @psalm-pure
+ *
+ * @psalm-flow ($str) -> return
+ */
+function trim(string $str, string $character_mask = " \t\n\r\0\x0B") : string {}
+
+/**
+ * @psalm-pure
+ *
+ * @psalm-flow ($str) -> return
+ */
+function ltrim(string $str, string $character_mask = " \t\n\r\0\x0B") : string {}
+
+/**
+ * @psalm-pure
+ *
+ * @psalm-flow ($str) -> return
+ */
+function rtrim(string $str, string $character_mask = " \t\n\r\0\x0B") : string {}
+
 /**
  * @psalm-pure
  *
@@ -493,10 +514,18 @@ function strtoupper(string $str) : string {}
  *     : string
  * )
  *
- * @psalm-flow ($glue, $pieces) -> return
+ * @psalm-flow ($glue) -> return
+ * @psalm-flow ($pieces) -(array-fetch)-> return
  */
 function implode($glue, array $pieces = []) : string {}
 
+/**
+ * @psalm-pure
+ *
+ * @psalm-flow ($string) -(array-assignment)-> return
+ */
+function explode(string $delimiter, string $string, int $limit = -1) : array {}
+
 /**
  * @param array $input
  *
@@ -514,15 +543,15 @@ function array_sum(array $input) {}
 /**
  * @psalm-pure
  *
- * @psalm-taint-remove html
+ * @psalm-taint-escape html
  * @psalm-flow ($str) -> return
  */
 function strip_tags(string $str, ?string $allowable_tags = null) : string {}
 
 /**
  * @psalm-pure
  *
- * @psalm-taint-remove html
+ * @psalm-taint-escape html
  *
  * @psalm-flow ($string) -> return
  */

diff --git a/src/Psalm/Storage/FunctionLikeStorage.php b/src/Psalm/Storage/FunctionLikeStorage.php
@@ -205,7 +205,7 @@ abstract class FunctionLikeStorage
     public $removed_taints = [];
 
     /**
-     * @var list<int>
+     * @var array<int, string>
      */
     public $return_source_params = [];
 

diff --git a/tests/TaintTest.php b/tests/TaintTest.php
@@ -1698,4 +1698,23 @@ public static function deleteUser(PDO $pdo, string $userId) : void {
 
         $this->analyzeFile('somefile.php', new Context());
     }
+
+    public function testImplodeExplode() : void
+    {
+        $this->expectException(\Psalm\Exception\CodeException::class);
+        $this->expectExceptionMessage('TaintedInput');
+
+        $this->project_analyzer->trackTaintedInputs();
+
+        $this->addFile(
+            'somefile.php',
+            '<?php
+                $a = $_GET["name"];
+                $b = explode(" ", $a);
+                $c = implode(" ", $b);
+                echo $c;'
+        );
+
+        $this->analyzeFile('somefile.php', new Context());
+    }
 }