fix(regex_redos): don't flag ? quantifier as nested quantifier vulnerability

The ? quantifier (0 or 1 times) cannot cause exponential backtracking
when used as the outer quantifier because it matches at most once.
Only unbounded quantifiers (+, *, {n,m} where m>1) should be flagged.

This fixes false positives for patterns like:
- ^/([_0-9a-zA-Z-]+/)?core/cache/...
- ^/([a-z]+/)?foo/bar

The change modifies the condition from checking if max > min (which is
true for ?) to checking if max > 1 (which excludes ?).

Fixes false positives reported in user configs with WP Rocket caching
rules that use optional prefix patterns.

Author: dvershinin

gixy/plugins/regex_redos.py

@@ -125,12 +125,15 @@ def _check_nested_quantifiers(self, parsed, depth, in_quantifier):
             if op in QUANTIFIERS:
                 min_repeat, max_repeat, subpattern = av
 
-                # Check if this quantifier can match variable length
-                can_repeat = (
-                    max_repeat > min_repeat or max_repeat == sre_parse.MAXREPEAT
+                # Check if this quantifier can match MULTIPLE times (more than once)
+                # The ? quantifier (max=1) cannot cause exponential backtracking as
+                # the outer quantifier because it matches at most once.
+                # Only unbounded quantifiers (+, *, {n,m} where m>1) are dangerous.
+                can_repeat_multiple = (
+                    max_repeat > 1 or max_repeat == sre_parse.MAXREPEAT
                 )
 
-                if can_repeat:
+                if can_repeat_multiple:
                     if in_quantifier:
                         # Found nested quantifier!
                         self.vulnerabilities.append(
@@ -158,7 +161,11 @@ def _check_nested_quantifiers(self, parsed, depth, in_quantifier):
                         subpattern, depth + 1, in_quantifier=True
                     )
                 else:
-                    self._check_nested_quantifiers(subpattern, depth + 1, in_quantifier)
+                    # Bounded quantifiers like ? (max=1) or {1,2} don't propagate
+                    # the in_quantifier context since they can't cause exponential backtracking
+                    self._check_nested_quantifiers(
+                        subpattern, depth + 1, in_quantifier=False
+                    )
 
             elif op == SUBPATTERN:
                 _, _, _, subpattern = av
@@ -175,11 +182,13 @@ def _check_nested_quantifiers(self, parsed, depth, in_quantifier):
                 self._check_nested_quantifiers(subpattern, depth + 1, in_quantifier)
 
     def _contains_quantifier(self, parsed):
-        """Check if parsed pattern contains any unbounded quantifiers."""
+        """Check if parsed pattern contains any unbounded quantifiers (can match more than once)."""
         for op, av in parsed:
             if op in QUANTIFIERS:
                 min_repeat, max_repeat, _ = av
-                if max_repeat > min_repeat or max_repeat == sre_parse.MAXREPEAT:
+                # Only unbounded quantifiers (+, *, {n,m} where m>1) count as dangerous
+                # The ? quantifier (max=1) is bounded and not dangerous for nesting
+                if max_repeat > 1 or max_repeat == sre_parse.MAXREPEAT:
                     return True
             elif op == SUBPATTERN:
                 _, _, _, subpattern = av

tests/plugins/simply/regex_redos/optional_group_fp.conf

@@ -0,0 +1,12 @@
+# fp: The ? quantifier (optional group) should not trigger nested quantifier detection
+# since it can only match 0 or 1 times - no exponential backtracking possible
+
+server {
+    location ~ ^/([_0-9a-zA-Z-]+/)?core/cache/busting/(.*)$ {
+        # This pattern has:
+        # - Outer ? (0 or 1 times) - bounded, not dangerous
+        # - Inner + on char class
+        # Not a nested quantifier vulnerability!
+        return 200;
+    }
+}

tests/plugins/test_redos_analyzer.py

@@ -141,6 +141,39 @@ def test_real_world_safe_pattern(self):
         vulns = analyzer.analyze()
         assert len(vulns) == 0
 
+    def test_optional_group_with_inner_quantifier(self):
+        """([a-z]+/)? is safe - ? means 0 or 1 times, no exponential backtracking."""
+        analyzer = RedosAnalyzer("^/([a-z]+/)?foo$")
+        vulns = analyzer.analyze()
+        assert len(vulns) == 0
+
+    def test_optional_group_real_world(self):
+        """Real-world cache busting pattern should be safe."""
+        analyzer = RedosAnalyzer("^/([_0-9a-zA-Z-]+/)?core/cache/busting/1/core/(.*)")
+        vulns = analyzer.analyze()
+        assert len(vulns) == 0
+
+    def test_optional_group_complex(self):
+        """Complex optional group should be safe."""
+        analyzer = RedosAnalyzer("^/(prefix-[a-z0-9]+)?/api/v[0-9]+$")
+        vulns = analyzer.analyze()
+        assert len(vulns) == 0
+
+    def test_bounded_outer_with_unbounded_inner(self):
+        """(a+){0,1} is same as (a+)? - bounded outer, should be safe."""
+        analyzer = RedosAnalyzer("(a+){0,1}")
+        vulns = analyzer.analyze()
+        assert len(vulns) == 0
+
+    def test_bounded_outer_max_2(self):
+        """(a+){1,2} has bounded outer max=2, still could have some backtracking."""
+        # This is a borderline case - max=2 means at most 2 repetitions
+        # Not exponential but could flag as nested
+        analyzer = RedosAnalyzer("(a+){1,2}")
+        vulns = analyzer.analyze()
+        # We allow max>1 to be flagged, so this should be flagged
+        assert len(vulns) > 0
+
 
 class TestRealWorldVulnerablePatterns:
     """Test detection of real-world vulnerable patterns."""