Skip to content

Commit 33b2701

Browse files
committed
refine isFork to remove false positive when a state has epsilon transition to itself
1 parent d7b22e3 commit 33b2701

File tree

3 files changed

+20
-3
lines changed

3 files changed

+20
-3
lines changed

javascript/ql/src/Performance/ReDoS.ql

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -705,7 +705,25 @@ predicate isFork(State q, InputSymbol s1, InputSymbol s2, State r1, State r2) {
705705
or
706706
r1 = r2 and
707707
q1 = q2 and
708-
epsilonSucc+(q) = q
708+
epsilonSucc+(q) = q and
709+
exists(RegExpTerm term | term = q.getRepr() | term instanceof InfiniteRepetitionQuantifier) and
710+
(
711+
// There is either multiple possible "mid" states.
712+
count(State mid |
713+
mid = epsilonSucc+(q) and
714+
q = epsilonSucc+(mid) and
715+
not mid = q
716+
) > 2
717+
or
718+
// Or one of the mid states is an infinite quantifier itself
719+
exists(State mid, RegExpTerm term |
720+
mid = epsilonSucc+(q) and
721+
q = epsilonSucc+(mid) and
722+
not mid = q and
723+
term = mid.getRepr() and
724+
term instanceof InfiniteRepetitionQuantifier
725+
)
726+
)
709727
) and
710728
stateInsideBacktracking(r1) and
711729
stateInsideBacktracking(r2)

javascript/ql/test/query-tests/Performance/ReDoS/ReDoS.expected

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,6 @@
133133
| tst.js:317:18:317:23 | [\\w-]* | This part of the regular expression may cause exponential backtracking on strings starting with 'foo' and containing many repetitions of '-'. |
134134
| tst.js:320:15:320:19 | (ab)* | This part of the regular expression may cause exponential backtracking on strings containing many repetitions of 'ab'. |
135135
| tst.js:323:14:323:20 | (a?a?)* | This part of the regular expression may cause exponential backtracking on strings containing many repetitions of 'a'. |
136-
| tst.js:326:15:326:19 | (a?)* | This part of the regular expression may cause exponential backtracking on strings containing many repetitions of 'a'. |
137136
| tst.js:329:14:329:20 | (c?a?)* | This part of the regular expression may cause exponential backtracking on strings containing many repetitions of 'a'. |
138137
| tst.js:332:14:332:22 | (?:a\|a?)+ | This part of the regular expression may cause exponential backtracking on strings containing many repetitions of 'a'. |
139138
| tst.js:335:14:335:20 | (a?b?)* | This part of the regular expression may cause exponential backtracking on strings containing many repetitions of 'a'. |

javascript/ql/test/query-tests/Performance/ReDoS/tst.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ var bad70 = /((ab)*)+c/;
322322
// NOT GOOD
323323
var bad71 = /(a?a?)*b/;
324324

325-
// GOOD - but still flagged. only quadratic blowup. (The NFA looks very similar to `/(a*)*b/`)
325+
// GOOD
326326
var good38 = /(a?)*b/;
327327

328328
// NOT GOOD - but wrong pump string.

0 commit comments

Comments
 (0)