Skip to content

Commit f24d7c4

Browse files
committed
Acknowledge new FPs due to the extractor using U+FFFD for unpaired surrogates
These were already misinterpreted, but the ReDoS code ignored them as they previously appeared to be `?` characters.
1 parent 487ebdf commit f24d7c4

File tree

4 files changed

+15
-12
lines changed

4 files changed

+15
-12
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,8 @@
145145
| tst.js:257:14:257:116 | (.thisisagoddamnlongstringforstresstestingthequery\|\\sthisisagoddamnlongstringforstresstestingthequery)* | This part of the regular expression may cause exponential backtracking on strings containing many repetitions of ' thisisagoddamnlongstringforstresstestingthequery'. |
146146
| tst.js:260:14:260:77 | (thisisagoddamnlongstringforstresstestingthequery\|this\\w+query)* | This part of the regular expression may cause exponential backtracking on strings containing many repetitions of 'thisisagoddamnlongstringforstresstestingthequery'. |
147147
| tst.js:260:68:260:70 | \\w+ | This part of the regular expression may cause exponential backtracking on strings starting with 'this' and containing many repetitions of 'aquerythis'. |
148+
| tst.js:266:18:266:49 | ([\\uDC66\\uDC67]\|[\\uDC68\\uDC69])* | This part of the regular expression may cause exponential backtracking on strings starting with 'foo' and containing many repetitions of '\ufffd'. |
149+
| tst.js:269:18:269:51 | ((\\uDC66\|\\uDC67)\|(\\uDC68\|\\uDC69))* | This part of the regular expression may cause exponential backtracking on strings starting with 'foo' and containing many repetitions of '\ufffd'. |
148150
| tst.js:272:21:272:22 | b+ | This part of the regular expression may cause exponential backtracking on strings containing many repetitions of 'b'. |
149151
| tst.js:275:38:275:40 | \\s* | This part of the regular expression may cause exponential backtracking on strings starting with '<a a=' and containing many repetitions of '"" a='. |
150152
| tst.js:281:16:281:17 | a+ | 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: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -262,10 +262,10 @@ var bad61 = /(thisisagoddamnlongstringforstresstestingthequery|this\w+query)*-/
262262
// GOOD
263263
var good27 = /(thisisagoddamnlongstringforstresstestingthequery|imanotherbutunrelatedstringcomparedtotheotherstring)*-/
264264

265-
// GOOD
265+
// GOOD (but false positive caused by the extractor converting all four unpaired surrogates to \uFFFD)
266266
var good28 = /foo([\uDC66\uDC67]|[\uDC68\uDC69])*foo/
267267

268-
// GOOD
268+
// GOOD (but false positive caused by the extractor converting all four unpaired surrogates to \uFFFD)
269269
var good29 = /foo((\uDC66|\uDC67)|(\uDC68|\uDC69))*foo/
270270

271271
// NOT GOOD (but cannot currently construct a prefix)
@@ -304,10 +304,10 @@ var bad66 = /^ab(c+)+$/;
304304
// NOT GOOD
305305
var bad67 = /(\d(\s+)*){20}/;
306306

307-
// GOOD - but we spuriously conclude that a rejecting suffix exists.
307+
// GOOD - but we spuriously conclude that a rejecting suffix exists.
308308
var good36 = /(([^/]|X)+)(\/[^]*)*$/;
309309

310-
// GOOD - but we spuriously conclude that a rejecting suffix exists.
310+
// GOOD - but we spuriously conclude that a rejecting suffix exists.
311311
var good37 = /^((x([^Y]+)?)*(Y|$))/;
312312

313313
// NOT GOOD
@@ -331,7 +331,7 @@ var bad72 = /(c?a?)*b/;
331331
// NOT GOOD
332332
var bad73 = /(?:a|a?)+b/;
333333

334-
// NOT GOOD - but not detected.
334+
// NOT GOOD - but not detected.
335335
var bad74 = /(a?b?)*$/;
336336

337337
// NOT GOOD
@@ -357,13 +357,13 @@ var good40 = /(a|b)+/;
357357
var good41 = /(?:[\s;,"'<>(){}|[\]@=+*]|:(?![/\\]))+/;
358358

359359
// NOT GOOD
360-
var bad83 = /^((?:a{|-)|\w\{)+X$/;
361-
var bad84 = /^((?:a{0|-)|\w\{\d)+X$/;
362-
var bad85 = /^((?:a{0,|-)|\w\{\d,)+X$/;
363-
var bad86 = /^((?:a{0,2|-)|\w\{\d,\d)+X$/;
360+
var bad83 = /^((?:a{|-)|\w\{)+X$/;
361+
var bad84 = /^((?:a{0|-)|\w\{\d)+X$/;
362+
var bad85 = /^((?:a{0,|-)|\w\{\d,)+X$/;
363+
var bad86 = /^((?:a{0,2|-)|\w\{\d,\d)+X$/;
364364

365-
// GOOD:
366-
var good42 = /^((?:a{0,2}|-)|\w\{\d,\d\})+X$/;
365+
// GOOD:
366+
var good42 = /^((?:a{0,2}|-)|\w\{\d,\d\})+X$/;
367367

368368
// GOOD
369369
var good43 = /("[^"]*?"|[^"\s]+)+(?=\s*|\s*$)/g;

javascript/ql/test/query-tests/RegExp/DuplicateCharacterInCharacterClass/DuplicateCharacterInCharacterClass.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
| tst.js:1:4:1:4 | o | Character 'o' is repeated $@ in the same character class. | tst.js:1:5:1:5 | o | here |
2+
| tst.js:3:3:3:8 | \\uDC3A | Character '\\uDC3A' is repeated $@ in the same character class. | tst.js:3:9:3:14 | \\uDC3C | here |
23
| tst.js:4:3:4:3 | ? | Character '?' is repeated $@ in the same character class. | tst.js:4:4:4:4 | ? | here |
34
| tst.js:5:3:5:8 | \\u003F | Character '\\u003F' is repeated $@ in the same character class. | tst.js:5:9:5:14 | \\u003f | here |
45
| tst.js:6:3:6:8 | \\u003F | Character '\\u003F' is repeated $@ in the same character class. | tst.js:6:9:6:9 | ? | here |

javascript/ql/test/query-tests/RegExp/DuplicateCharacterInCharacterClass/tst.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/[foo]/;
22
/[a-zc]/;
3-
/[\uDC3A\uDC3C]/;
3+
/[\uDC3A\uDC3C]/; // False positive caused by the extractor converting both unpaired surrogates to \uFFFD
44
/[??]/;
55
/[\u003F\u003f]/;
66
/[\u003F?]/;

0 commit comments

Comments
 (0)