BAP-22681: Update SQL Injection test dependencies to work with PHP 8.3

oroinc · Jul 10, 2024 · f7eceb7 · f7eceb7
1 parent 449473f
commit f7eceb7
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 15 deletions.
diff --git a/extension.neon b/extension.neon
@@ -83,7 +83,7 @@ parametersSchema:
             safe_methods: arrayOf(arrayOf(anyOf(bool(), arrayOf(int()))))
             check_static_methods_safety: arrayOf(arrayOf(anyOf(bool(), arrayOf(int()))))
             check_methods_safety: arrayOf(arrayOf(anyOf(bool(), arrayOf(int()))))
-            check_methods: arrayOf(arrayOf(anyOf(bool(), arrayOf(int()))))
+            check_methods: arrayOf(arrayOf(anyOf(bool(), arrayOf(anyOf(int(), string())))))
             clear_static_methods: arrayOf(arrayOf(bool()))
             clear_methods: arrayOf(arrayOf(bool()))
         ])

diff --git a/rules.neon b/rules.neon
@@ -60,6 +60,7 @@ parameters:
                     getIdentifier: true
                     getTableName: true
                     getAssociationMapping: true
+                    getColumnNames: true
 
                 Doctrine\ORM\Mapping\ClassMetadataInfo:
                     getSingleIdentifierFieldName: true
@@ -107,6 +108,9 @@ parameters:
                 Doctrine\Common\Collections\ExpressionBuilder:
                     notExists: [0]
 
+                Doctrine\DBAL\Platforms\AbstractPlatform:
+                    getTruncateTableSQL: [0]
+
             check_methods:
                 Doctrine\ORM\QueryBuilder:
                     __all__: true
@@ -172,15 +176,20 @@ parameters:
 
                 Doctrine\DBAL\Connection:
                     fetchAssoc: [0]
+                    fetchAssociative: [0]
+                    fetchAllAssociative: [0]
+                    fetchNumeric: [0]
+                    fetchOne: [0]
                     fetchArray: [0]
                     fetchColumn: [0]
                     fetchAll: [0]
-                    delete: [0, 1]
-                    update: [0, 1, 2]
-                    insert: [0, 1]
+                    delete: [0, 1:keys]
+                    update: [0, 1:keys, 2:keys]
+                    insert: [0, 1:keys]
                     prepare: true
                     executeQuery: [0]
                     executeCacheQuery: [0]
+                    executeStatement: [0]
                     project: [0]
                     query: [0, 1]
                     executeUpdate: [0]
@@ -200,6 +209,7 @@ parameters:
                     getSingleAssociationJoinColumnName: true
                     getSingleAssociationReferencedJoinColumnName: true
                     getFieldForColumn: true
+                    getColumnName: true
 
                 Doctrine\Persistence\ManagerRegistry:
                     getManagerForClass: true

diff --git a/src/Oro/Rules/Methods/QueryBuilderInjectionRule.php b/src/Oro/Rules/Methods/QueryBuilderInjectionRule.php
@@ -33,6 +33,7 @@ class QueryBuilderInjectionRule implements \PHPStan\Rules\Rule
 
     const SAFE_FUNCTIONS = [
         'base64_encode' => true,
+        'count' => true
     ];
 
     const VAR = 'variables';
@@ -237,6 +238,7 @@ private function isUnsafeMethodCall(Node\Expr $value, Scope $scope, array &$erro
                         $scope->getFunctionName()
                     );
                 }
+
                 return true;
             }
 
@@ -284,7 +286,7 @@ private function isUnsafeMethodCall(Node\Expr $value, Scope $scope, array &$erro
                 ) !== null
                 ||
                 (
-                    $result = $this
+                $result = $this
                     ->checkMethodArguments(
                         $value,
                         $scope,
@@ -321,9 +323,20 @@ private function checkMethodArguments(
         bool $isConstructor = false
     ) {
         if (isset($config[$className])) {
-            $methodName = $isConstructor ? '__construct': (string)$value->name;
+            $methodName = $isConstructor ? '__construct' : (string)$value->name;
             $checkArg = function ($pos, array &$errors = []) use ($className, $value, $scope, $methodName) {
-                if ($this->isUnsafe($value->args[$pos]->value, $scope)) {
+                $checkKeys = true;
+                $checkValues = true;
+                if (is_string($pos) && str_contains($pos, ':')) {
+                    [$pos, $type] = explode(':', $pos);
+                    if ($type === 'keys') {
+                        $checkValues = false;
+                    }
+                    if ($type === 'values') {
+                        $checkKeys = false;
+                    }
+                }
+                if ($this->isUnsafe($value->args[$pos]->value, $scope, $checkKeys, $checkValues)) {
                     $errors[] = \sprintf(
                         'Unsafe calling method %s::%s. ' . PHP_EOL .
                         'Argument %d contains unsafe values %s. ' . PHP_EOL .
@@ -398,6 +411,7 @@ private function isUnsafeNew(Node\Expr $value, Scope $scope)
         }
 
         $errors = [];
+
         return $this->checkMethodArguments(
             $value,
             $scope,
@@ -537,13 +551,19 @@ private function isUnsafeArrayDimFetch(Node\Expr $value, Scope $scope): bool
      * @param Scope $scope
      * @return bool
      */
-    private function isUnsafeArray(Node\Expr $value, Scope $scope): bool
-    {
+    private function isUnsafeArray(
+        Node\Expr $value,
+        Scope $scope,
+        bool $checkKeys = true,
+        bool $checkValues = true
+    ): bool {
         if ($value instanceof Node\Expr\Array_) {
             foreach ($value->items as $arrayItem) {
-                if (($arrayItem->key && $this->isUnsafe($arrayItem->key, $scope))
-                    || $this->isUnsafe($arrayItem->value, $scope)
-                ) {
+                if ($checkKeys && $arrayItem->key && $this->isUnsafe($arrayItem->key, $scope)) {
+                    return true;
+                }
+
+                if ($checkValues && $this->isUnsafe($arrayItem->value, $scope)) {
                     return true;
                 }
             }
@@ -598,8 +618,12 @@ private function isUnsafeTernary(Node\Expr $value, Scope $scope): bool
      * @param Scope $scope
      * @return bool
      */
-    private function isUnsafe(Node\Expr $value, Scope $scope): bool
-    {
+    private function isUnsafe(
+        Node\Expr $value,
+        Scope $scope,
+        bool $checkKeys = true,
+        bool $checkValues = true
+    ): bool {
         return $this->isUncheckedType($value)
             || $this->isUnsafeNew($value, $scope)
             || $this->isUnsafeVariable($value, $scope)
@@ -611,7 +635,7 @@ private function isUnsafe(Node\Expr $value, Scope $scope): bool
             || $this->isUnsafeConcat($value, $scope)
             || $this->isUnsafeEncapsedString($value, $scope)
             || $this->isUnsafeCast($value, $scope)
-            || $this->isUnsafeArray($value, $scope)
+            || $this->isUnsafeArray($value, $scope, $checkKeys, $checkValues)
             || $this->isUnsafeTernary($value, $scope);
     }