Merge pull request #18892 from asgerf/js/membership-regexp-test

JS: Sharpen up EnumerationRegExp

Author: erik-krogh

javascript/ql/lib/semmle/javascript/MembershipCandidates.qll

@@ -140,22 +140,17 @@ module MembershipCandidate {
     EnumerationRegExp() {
       this.isRootTerm() and
       RegExp::isFullyAnchoredTerm(this) and
-      exists(RegExpTerm child | this.getAChild*() = child |
-        child instanceof RegExpSequence or
-        child instanceof RegExpCaret or
-        child instanceof RegExpDollar or
-        child instanceof RegExpConstant or
-        child instanceof RegExpAlt or
-        child instanceof RegExpGroup
-      ) and
-      // exclude "length matches" that match every string
-      not this.getAChild*() instanceof RegExpDot
+      not exists(RegExpTerm child | child.getRootTerm() = this |
+        child instanceof RegExpDot or
+        child instanceof RegExpCharacterClass or
+        child instanceof RegExpUnicodePropertyEscape
+      )
     }
 
     /**
      * Gets a string matched by this regular expression.
      */
-    string getAMember() { result = this.getAChild*().getAMatchedString() }
+    string getAMember() { result = any(RegExpTerm t | t.getRootTerm() = this).getAMatchedString() }
   }
 
   /**

javascript/ql/src/change-notes/2025-02-28-membership-regexp-test.md

@@ -0,0 +1,7 @@
+---
+category: fix
+---
+* Fixed a bug that would in rare cases cause some regexp-based checks
+  to be seen as generic taint sanitisers, even though the underlying regexp
+  is not restrictive enough. The regexps are now analysed more precisely,
+  and unrestrictive regexp checks will no longer block taint flow.

javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected

@@ -238,6 +238,7 @@ flow
 | promise.js:18:22:18:29 | source() | promise.js:24:10:24:10 | e |
 | promise.js:33:21:33:28 | source() | promise.js:38:10:38:10 | e |
 | promise.js:43:20:43:27 | source() | promise.js:43:8:43:28 | Promise ... urce()) |
+| regexp-sanitiser.js:2:19:2:26 | source() | regexp-sanitiser.js:4:14:4:18 | taint |
 | rxjs.js:3:1:3:8 | source() | rxjs.js:10:14:10:17 | data |
 | rxjs.js:13:1:13:8 | source() | rxjs.js:17:23:17:23 | x |
 | rxjs.js:13:1:13:8 | source() | rxjs.js:18:23:18:23 | x |

javascript/ql/test/library-tests/TaintTracking/DataFlowTracking.expected

@@ -161,6 +161,7 @@ flow
 | partialCalls.js:4:17:4:24 | source() | partialCalls.js:30:14:30:20 | x.value |
 | partialCalls.js:4:17:4:24 | source() | partialCalls.js:41:10:41:18 | id(taint) |
 | partialCalls.js:4:17:4:24 | source() | partialCalls.js:51:14:51:14 | x |
+| regexp-sanitiser.js:2:19:2:26 | source() | regexp-sanitiser.js:4:14:4:18 | taint |
 | sanitizer-function.js:12:17:12:24 | source() | sanitizer-function.js:14:10:14:14 | taint |
 | sanitizer-function.js:12:17:12:24 | source() | sanitizer-function.js:17:14:17:18 | taint |
 | sanitizer-function.js:12:17:12:24 | source() | sanitizer-function.js:21:14:21:18 | taint |

javascript/ql/test/library-tests/TaintTracking/regexp-sanitiser.js

@@ -0,0 +1,6 @@
+function foo() {
+    const taint = source();
+    if (/^asd[\s\S]*$/.test(taint)) {
+        sink(taint); // NOT OK
+    }
+}