JS: Now BadHtmlSanitizers new RegExp with unknown flags is also flagged.

Author: Napalys

javascript/ql/lib/semmle/javascript/StandardLibrary.qll

@@ -117,6 +117,14 @@ class StringReplaceCall extends DataFlow::MethodCallNode {
    */
   predicate isGlobal() { this.getRegExp().isGlobal() or this.getMethodName() = "replaceAll" }
 
+  /**
+   * Holds if this is a global replacement, that is, the first argument is a regular expression
+   * with the `g` flag, or this is a call to `.replaceAll()`.
+   */
+  predicate maybeGlobal() {
+    RegExp::maybeGlobal(this.getRegExp().tryGetFlags()) or this.getMethodName() = "replaceAll"
+  }
+
   /**
    * Holds if this call to `replace` replaces `old` with `new`.
    */

javascript/ql/lib/semmle/javascript/security/IncompleteBlacklistSanitizer.qll

@@ -74,7 +74,7 @@ private StringReplaceCall getAStringReplaceMethodCall(StringReplaceCall n) {
 module HtmlSanitization {
   private predicate fixedGlobalReplacement(StringReplaceCallSequence chain) {
     forall(StringReplaceCall member | member = chain.getAMember() |
-      member.isGlobal() and member.getArgument(0) instanceof DataFlow::RegExpCreationNode
+      member.maybeGlobal() and member.getArgument(0) instanceof DataFlow::RegExpCreationNode
     )
   }
 

javascript/ql/test/query-tests/Security/CWE-116/IncompleteSanitization/IncompleteBlacklistSanitizer.expected

@@ -68,3 +68,6 @@
 | tst.js:333:2:333:40 | s().rep ... g"),'') | This HTML sanitizer does not sanitize ampersands |
 | tst.js:333:2:333:40 | s().rep ... g"),'') | This HTML sanitizer does not sanitize double quotes |
 | tst.js:333:2:333:40 | s().rep ... g"),'') | This HTML sanitizer does not sanitize single quotes |
+| tst.js:337:2:337:46 | s().rep ... ()),'') | This HTML sanitizer does not sanitize ampersands |
+| tst.js:337:2:337:46 | s().rep ... ()),'') | This HTML sanitizer does not sanitize double quotes |
+| tst.js:337:2:337:46 | s().rep ... ()),'') | This HTML sanitizer does not sanitize single quotes |

javascript/ql/test/query-tests/Security/CWE-116/IncompleteSanitization/tst.js

@@ -334,5 +334,5 @@ function typicalBadHtmlSanitizers(s) {
 }
 
 function typicalBadHtmlSanitizers(s) {
-	s().replace(new RegExp("[<>]", unknown()),''); // NOT OK -- should be flagged, because it is st ill a bad sanitizer
+	s().replace(new RegExp("[<>]", unknown()),''); // NOT OK
 }