Skip to content

Commit 40d37b1

Browse files
committed
fix: detect (a+)+ as vulnerable in AUTO mode (issue #2)
The pattern (a+)+ was incorrectly reported as safe due to flawed logic in _is_multi_trans_exploitable that assumed unanchored patterns could always "escape early". This is only true for optional quantifiers like (a*)*, not for required quantifiers like (a+)+. Changed AUTO mode to be conservative by always reporting multi-transitions as exploitable. Users who want lenient analysis can use match_mode=PARTIAL. Added regression test for issue #2.
1 parent 76daeac commit 40d37b1

File tree

2 files changed

+24
-8
lines changed

2 files changed

+24
-8
lines changed

src/redoctor/automaton/scc_checker.py

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -326,14 +326,9 @@ def _is_multi_trans_exploitable(self, state: NFAState, nfa_char: NFAChar) -> boo
326326
# Has end anchor OR requires continuation → exploitable
327327
return True
328328

329-
# AUTO mode: use anchor and continuation detection
330-
if not self.has_end_anchor and not self.requires_continuation:
331-
# No end anchor and no continuation → can escape early → not exploitable
332-
# This is the key insight: (a*)* without $ is safe in partial match
333-
# But ^([^@]+)+@ IS exploitable because of the @ after the quantifier
334-
return False
335-
336-
# Has end anchor OR requires continuation → must try all combinations → exploitable
329+
# AUTO mode: conservative approach for security analysis
330+
# Multi-transitions indicate nested quantifier ambiguity - report as exploitable
331+
# Users who want lenient analysis can use match_mode=PARTIAL
337332
return True
338333

339334
def _check_eda_with_pair_graph(

tests/test_regressions.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,3 +114,24 @@ def test_username_pattern(self):
114114
result = check(pattern, config=Config.quick())
115115
assert result is not None
116116
assert result.status.value in ("safe", "unknown")
117+
118+
119+
class TestGitHubIssues:
120+
"""Regression tests for reported GitHub issues."""
121+
122+
def test_issue_2_unanchored_nested_plus_is_vulnerable(self):
123+
"""GitHub issue #2: (a+)+ should be detected as vulnerable.
124+
125+
The pattern (a+)+ without anchors was incorrectly reported as safe
126+
with O(n) complexity. It should be detected as vulnerable with
127+
exponential complexity due to nested quantifier ambiguity.
128+
129+
Note: We use skip_recall=True because recall validation uses re.match()
130+
which doesn't require full-string matching, so the attack string doesn't
131+
trigger backtracking. The automaton analysis correctly identifies this
132+
as vulnerable in full-match contexts (e.g., re.fullmatch()).
133+
"""
134+
result = check(r"(a+)+", config=Config(skip_recall=True))
135+
assert result.is_vulnerable
136+
assert result.complexity is not None
137+
assert result.complexity.is_exponential

0 commit comments

Comments
 (0)