From 55f2f86a26e08aac6ff35e239e30103fee1ed9a8 Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Tue, 17 Nov 2020 21:15:32 +0100 Subject: [PATCH] limit the search of state-pairs to the ones that are reachable within the given length --- javascript/ql/src/Performance/ReDoS.ql | 4 +++- .../ReDoS/PolynomialBackTracking.expected | 5 +++++ .../query-tests/Performance/ReDoS/ReDoS.expected | 3 +++ .../ql/test/query-tests/Performance/ReDoS/tst.js | 14 +++++++++++++- 4 files changed, 24 insertions(+), 2 deletions(-) diff --git a/javascript/ql/src/Performance/ReDoS.ql b/javascript/ql/src/Performance/ReDoS.ql index 55765e378c5..cc0516fb24a 100644 --- a/javascript/ql/src/Performance/ReDoS.ql +++ b/javascript/ql/src/Performance/ReDoS.ql @@ -799,6 +799,7 @@ string concretise(Trace t) { * a path from `r` back to `(fork, fork)` with `rem` steps. */ predicate isReachableFromFork(State fork, StatePair r, Trace w, int rem) { + // base case exists(InputSymbol s1, InputSymbol s2, State q1, State q2 | isFork(fork, s1, s2, q1, q2) and r = MkStatePair(q1, q2) and @@ -806,11 +807,12 @@ predicate isReachableFromFork(State fork, StatePair r, Trace w, int rem) { rem = statePairDist(r, MkStatePair(fork, fork)) ) or + // recursive case exists(StatePair p, Trace v, InputSymbol s1, InputSymbol s2 | isReachableFromFork(fork, p, v, rem + 1) and step(p, s1, s2, r) and w = Step(s1, s2, v) and - rem > 0 + rem >= statePairDist(r, MkStatePair(fork, fork)) ) } diff --git a/javascript/ql/test/query-tests/Performance/ReDoS/PolynomialBackTracking.expected b/javascript/ql/test/query-tests/Performance/ReDoS/PolynomialBackTracking.expected index 76020308d68..96871051fb0 100644 --- a/javascript/ql/test/query-tests/Performance/ReDoS/PolynomialBackTracking.expected +++ b/javascript/ql/test/query-tests/Performance/ReDoS/PolynomialBackTracking.expected @@ -263,3 +263,8 @@ | tst.js:251:18:251:19 | A* | it can start matching anywhere | | tst.js:251:18:251:19 | A* | it can start matching anywhere after the start of the preceeding 'A*' | | tst.js:260:14:260:21 | (\\n\\s*)+ | it can start matching anywhere | +| tst.js:266:14:266:91 | (\\w*foobarbaz\\w*foobarbaz\\w*foobarbaz\\w*foobarbaz\\s*foobarbaz\\d*foobarbaz\\w*)+ | it can start matching anywhere | +| tst.js:266:15:266:17 | \\w* | it can start matching anywhere | +| tst.js:269:14:269:116 | (.thisisagoddamnlongstringforstresstestingthequery\|\\sthisisagoddamnlongstringforstresstestingthequery)* | it can start matching anywhere | +| tst.js:272:14:272:77 | (thisisagoddamnlongstringforstresstestingthequery\|this\\w+query)* | it can start matching anywhere | +| tst.js:275:15:275:117 | (thisisagoddamnlongstringforstresstestingthequery\|imanotherbutunrelatedstringcomparedtotheotherstring)* | it can start matching anywhere | diff --git a/javascript/ql/test/query-tests/Performance/ReDoS/ReDoS.expected b/javascript/ql/test/query-tests/Performance/ReDoS/ReDoS.expected index f239c1c08f3..86ba3387bb3 100644 --- a/javascript/ql/test/query-tests/Performance/ReDoS/ReDoS.expected +++ b/javascript/ql/test/query-tests/Performance/ReDoS/ReDoS.expected @@ -113,3 +113,6 @@ | tst.js:254:17:254:21 | [^>]+ | This part of the regular expression may cause exponential backtracking on strings containing many repetitions of '='. | | tst.js:257:16:257:21 | [^>a]+ | This part of the regular expression may cause exponential backtracking on strings containing many repetitions of '='. | | tst.js:260:17:260:19 | \\s* | This part of the regular expression may cause exponential backtracking on strings containing many repetitions of '\\n'. | +| tst.js:266:87:266:89 | \\w* | This part of the regular expression may cause exponential backtracking on strings containing many repetitions of '0foobarbazfoobarbazfoobarbazfoobarbazfoobarbazfoobarbaz'. | +| tst.js:269:14:269:116 | (.thisisagoddamnlongstringforstresstestingthequery\|\\sthisisagoddamnlongstringforstresstestingthequery)* | This part of the regular expression may cause exponential backtracking on strings containing many repetitions of ' thisisagoddamnlongstringforstresstestingthequery'. | +| tst.js:272:14:272:77 | (thisisagoddamnlongstringforstresstestingthequery\|this\\w+query)* | This part of the regular expression may cause exponential backtracking on strings containing many repetitions of 'thisisagoddamnlongstringforstresstestingthequery'. | diff --git a/javascript/ql/test/query-tests/Performance/ReDoS/tst.js b/javascript/ql/test/query-tests/Performance/ReDoS/tst.js index d34237d5176..2e2585cf417 100644 --- a/javascript/ql/test/query-tests/Performance/ReDoS/tst.js +++ b/javascript/ql/test/query-tests/Performance/ReDoS/tst.js @@ -260,4 +260,16 @@ var bad57 = /^([^>a]+)*(>|$)/; var bad58 = /(\n\s*)+$/; // GOOD -var good26 = /([^\\\]]+)*/ \ No newline at end of file +var good26 = /([^\\\]]+)*/ + +// NOT GOOD +var bad59 = /(\w*foobarbaz\w*foobarbaz\w*foobarbaz\w*foobarbaz\s*foobarbaz\d*foobarbaz\w*)+-/; + +// NOT GOOD +var bad60 = /(.thisisagoddamnlongstringforstresstestingthequery|\sthisisagoddamnlongstringforstresstestingthequery)*-/ + +// NOT GOOD +var bad61 = /(thisisagoddamnlongstringforstresstestingthequery|this\w+query)*-/ + +// GOOD +var good27 = /(thisisagoddamnlongstringforstresstestingthequery|imanotherbutunrelatedstringcomparedtotheotherstring)*-/ \ No newline at end of file