support non-conditional @psalm-taint-escape sql (#437)

* support `@psalm-taint-escape sql` * cs * cs * Update PhpDocUtil.php
staabm · Oct 8, 2022 · 271f837 · 271f837
1 parent 59d5d8e
commit 271f837
Show file tree

Hide file tree

Showing 4 changed files with 85 additions and 14 deletions.
diff --git a/src/PhpDoc/PhpDocUtil.php b/src/PhpDoc/PhpDocUtil.php
@@ -13,10 +13,43 @@ final class PhpDocUtil
 {
     /**
      * @api
+     *
+     * @param CallLike|MethodReflection $callLike
+     */
+    public static function matchTaintEscape($callLike, Scope $scope): ?string
+    {
+        if ($callLike instanceof CallLike) {
+            $methodReflection = self::getMethodReflection($callLike, $scope);
+        } else {
+            $methodReflection = $callLike;
+        }
+
+        // XXX does not yet support conditional escaping
+        // https://psalm.dev/docs/security_analysis/avoiding_false_positives/#conditional-escaping-tainted-input
+        if (null !== $methodReflection) {
+            // atm no resolved phpdoc for methods
+            // see https://github.com/phpstan/phpstan/discussions/7657
+            $phpDocString = $methodReflection->getDocComment();
+            if (null !== $phpDocString && preg_match('/@psalm-taint-escape\s+(\S+)$/m', $phpDocString, $matches)) {
+                return $matches[1];
+            }
+        }
+
+        return null;
+    }
+
+    /**
+     * @param CallLike|MethodReflection $callLike
+     *
+     *@api
      */
-    public static function commentContains(string $text, CallLike $callike, Scope $scope): bool
+    public static function commentContains(string $text, $callLike, Scope $scope): bool
     {
-        $methodReflection = self::getMethodReflection($callike, $scope);
+        if ($callLike instanceof CallLike) {
+            $methodReflection = self::getMethodReflection($callLike, $scope);
+        } else {
+            $methodReflection = $callLike;
+        }
 
         if (null !== $methodReflection) {
             // atm no resolved phpdoc for methods
@@ -35,9 +68,9 @@ public static function commentContains(string $text, CallLike $callike, Scope $s
      *
      * @param string $annotation e.g. '@phpstandba-inference-placeholder'
      */
-    public static function matchStringAnnotation(string $annotation, CallLike $callike, Scope $scope): ?string
+    public static function matchStringAnnotation(string $annotation, CallLike $callLike, Scope $scope): ?string
     {
-        $methodReflection = self::getMethodReflection($callike, $scope);
+        $methodReflection = self::getMethodReflection($callLike, $scope);
 
         if (null !== $methodReflection) {
             // atm no resolved phpdoc for methods
@@ -57,21 +90,21 @@ public static function matchStringAnnotation(string $annotation, CallLike $calli
         return null;
     }
 
-    private static function getMethodReflection(CallLike $callike, Scope $scope): ?MethodReflection
+    private static function getMethodReflection(CallLike $callLike, Scope $scope): ?MethodReflection
     {
         $methodReflection = null;
-        if ($callike instanceof Expr\StaticCall) {
-            if ($callike->class instanceof Name && $callike->name instanceof Identifier) {
-                $classType = $scope->resolveTypeByName($callike->class);
-                $methodReflection = $scope->getMethodReflection($classType, $callike->name->name);
+        if ($callLike instanceof Expr\StaticCall) {
+            if ($callLike->class instanceof Name && $callLike->name instanceof Identifier) {
+                $classType = $scope->resolveTypeByName($callLike->class);
+                $methodReflection = $scope->getMethodReflection($classType, $callLike->name->name);
             }
-        } elseif ($callike instanceof Expr\MethodCall && $callike->name instanceof Identifier) {
+        } elseif ($callLike instanceof Expr\MethodCall && $callLike->name instanceof Identifier) {
             $classReflection = $scope->getClassReflection();
-            if (null !== $classReflection && $classReflection->hasMethod($callike->name->name)) {
-                $methodReflection = $classReflection->getMethod($callike->name->name, $scope);
+            if (null !== $classReflection && $classReflection->hasMethod($callLike->name->name)) {
+                $methodReflection = $classReflection->getMethod($callLike->name->name, $scope);
             } else {
-                $callerType = $scope->getType($callike->var);
-                $methodReflection = $scope->getMethodReflection($callerType, $callike->name->name);
+                $callerType = $scope->getType($callLike->var);
+                $methodReflection = $scope->getMethodReflection($callerType, $callLike->name->name);
             }
         }
 

diff --git a/src/QueryReflection/QueryReflection.php b/src/QueryReflection/QueryReflection.php
@@ -201,6 +201,10 @@ private function resolveQueryStringExpr(Expr $queryExpr, Scope $scope, bool $res
         }
 
         if ($queryExpr instanceof Expr\CallLike) {
+            if ('sql' === PhpDocUtil::matchTaintEscape($queryExpr, $scope)) {
+                return '1';
+            }
+
             $placeholder = PhpDocUtil::matchStringAnnotation('@phpstandba-inference-placeholder', $queryExpr, $scope);
 
             if (null !== $placeholder) {

diff --git a/tests/default/Fixture/Escaper.php b/tests/default/Fixture/Escaper.php
@@ -0,0 +1,20 @@
+<?php
+
+namespace staabm\PHPStanDba\Tests\Fixture;
+
+final class Escaper
+{
+    /**
+     * @psalm-taint-escape sql
+     */
+    public function escape($s): string
+    {
+    }
+
+    /**
+     * @psalm-taint-escape sql
+     */
+    public static function staticEscape($s): string
+    {
+    }
+}
diff --git a/tests/default/data/pdo.php b/tests/default/data/pdo.php
@@ -4,6 +4,7 @@
 
 use PDO;
 use function PHPStan\Testing\assertType;
+use staabm\PHPStanDba\Tests\Fixture\Escaper;
 
 class Foo
 {
@@ -224,4 +225,17 @@ public function queryEncapsedString(PDO $pdo, int $adaid)
         $stmt = $pdo->query("SELECT email, adaid FROM ada WHERE adaid={$fn()}", PDO::FETCH_ASSOC);
         assertType('PDOStatement<array{email: string, adaid: int<-32768, 32767>}>', $stmt);
     }
+
+    public function taintStaticEscaped(PDO $pdo, string $s)
+    {
+        $stmt = $pdo->query('SELECT email, adaid FROM ada WHERE adaid='.Escaper::staticEscape($s), PDO::FETCH_ASSOC);
+        assertType('PDOStatement<array{email: string, adaid: int<-32768, 32767>}>', $stmt);
+    }
+
+    public function taintEscaped(PDO $pdo, string $s)
+    {
+        $escapeer = new Escaper();
+        $stmt = $pdo->query('SELECT email, adaid FROM ada WHERE adaid='.$escapeer->escape($s), PDO::FETCH_ASSOC);
+        assertType('PDOStatement<array{email: string, adaid: int<-32768, 32767>}>', $stmt);
+    }
 }