#10: OR filter bypasses all doctrine extensions -> Potential security…

… problem - Applies workaround only with OR
metaclass-nl · Aug 10, 2022 · 79fb554 · 79fb554
1 parent e60889a
commit 79fb554
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 21 deletions.
diff --git a/src/Filter/FilterLogic.php b/src/Filter/FilterLogic.php
@@ -122,25 +122,24 @@ public function apply(QueryBuilder $queryBuilder, QueryNameGeneratorInterface $q
             $this->replaceInnerJoinsByLeftJoins($queryBuilder);
         }
 
-        // if $existingWhere empty no problem
-        // if  $filterWhere empty nest OR in an extra AND
-        if (empty($existingWhere) || empty($filterWhere) ) {
+        // if $existingWhere empty it does not matter how applied
+        // if combinator == AND no problem
+        // if  $filterWhere empty use andWhere
+        if (empty($existingWhere) || empty($filterWhere) || $combinator == 'AND') {
             $queryBuilder->andWhere($logicExp);
             return;
         }
         // elseif only criteria from filters, apply according to operator
         if ($existingWhere == $filterWhere) {
-            if ($combinator == 'OR') {
-                $queryBuilder->orWhere($logicExp);
-            } else {
-                $queryBuilder->andWhere($logicExp);
-            }
+            $queryBuilder->orWhere($logicExp);
             return;
         }
-        // elseif criteria from filters follow AND, replace them
+
+        // Criteria from both extensions and filters, should OR only with those from filters,
+        // replace them if criteria from filters follow AND
         if(false!==strpos($existingWhere, " AND $filterWhere")) {
             $queryBuilder->add('where',
-                str_replace($filterWhere, "($filterWhere $combinator ($logicExp))", $existingWhere)
+                str_replace($filterWhere, "($filterWhere OR ($logicExp))", $existingWhere)
             );
             return;
         }

diff --git a/tests/Filter/FilterLogicWithAnnotationTest.php b/tests/Filter/FilterLogicWithAnnotationTest.php
@@ -133,9 +133,10 @@ public function testDdFilterOr()
             'Parameter dd_p2');
     }
 
-    public function testDdFilterAndWithExtsionCriterium()
+    public function testDdFilterAndWithExtsionCriteria()
     {
-        $this->testEntityQb->andWhere('o.numb >= 0');
+        $this->testEntityQb->orWhere('o.numb >= 0');
+        $this->testEntityQb->orWhere('o.numb <= 999');
         $reqData = null;
         parse_str('exists[bool]=true&and[or][dd][after]=2021-01-01', $reqData);
         // var_dump($reqData);
@@ -147,10 +148,10 @@ public function testDdFilterAndWithExtsionCriterium()
         $this->assertEquals(
             str_replace('
 ', '', "SELECT o FROM Metaclass\FilterBundle\Entity\TestEntity o WHERE 
-o.numb >= 0 AND (
+(o.numb >= 0 OR o.numb <= 999) AND 
 o.bool IS NOT NULL 
 AND (o.dd >= :dd_p1 OR o.dd IS NULL)
-)"),
+"),
             $this->testEntityQb->getDQL(),
             'DQL');
         $this->assertEquals(
@@ -160,9 +161,10 @@ public function testDdFilterAndWithExtsionCriterium()
 
     }
 
-    public function testDdFilterNotWithExtsionCriterium()
+    public function testDdFilterNotWithExtsionCriteria()
     {
-        $this->testEntityQb->andWhere('o.numb >= 0');
+        $this->testEntityQb->orWhere('o.numb >= 0');
+        $this->testEntityQb->orWhere('o.numb <= 999');
         $reqData = null;
         parse_str('exists[bool]=true&not[dd][after]=2021-01-01', $reqData);
         // var_dump($reqData);
@@ -174,9 +176,9 @@ public function testDdFilterNotWithExtsionCriterium()
         $this->assertEquals(
             str_replace('
 ', '', "SELECT o FROM Metaclass\FilterBundle\Entity\TestEntity o WHERE 
-o.numb >= 0 AND (
+(o.numb >= 0 OR o.numb <= 999) AND 
 o.bool IS NOT NULL 
-AND (NOT(o.dd >= :dd_p1 OR o.dd IS NULL)))"),
+AND (NOT(o.dd >= :dd_p1 OR o.dd IS NULL))"),
             $this->testEntityQb->getDQL(),
             'DQL');
         $this->assertEquals(
@@ -185,9 +187,10 @@ public function testDdFilterNotWithExtsionCriterium()
             'Parameter dd_p1');
     }
 
-    public function testDdFilterOrWithExtsionCriterium()
+    public function testDdFilterOrWithExtsionCriteria()
     {
-        $this->testEntityQb->andWhere('o.numb >= 0');
+        $this->testEntityQb->orWhere('o.numb >= 0');
+        $this->testEntityQb->orWhere('o.numb <= 999');
         $reqData = null;
         parse_str('exists[bool]=true&or[dd][after]=2021-01-01&or[dd][before]=2010-02-02', $reqData);
         // var_dump($reqData);
@@ -199,7 +202,7 @@ public function testDdFilterOrWithExtsionCriterium()
         $this->assertEquals(
             str_replace('
 ', '', "SELECT o FROM Metaclass\FilterBundle\Entity\TestEntity o WHERE 
-o.numb >= 0 AND (
+(o.numb >= 0 OR o.numb <= 999) AND (
 o.bool IS NOT NULL 
 OR (
 (o.dd <= :dd_p1 AND o.dd IS NOT NULL)