Merge pull request github#18089 from Napalys/napalys/regexp-unknown-flags

JS: RegExp unknown flags support and enhanced compatibility with RegExp objects

Author: Napalys

javascript/ql/lib/change-notes/2024-11-28-regexp-unknown-flags.md

@@ -0,0 +1,6 @@
+---
+category: majorAnalysis
+---
+* The `js/incomplete-sanitization` query now also checks regular expressions constructed using `new RegExp(..)`. Previously it only checked regular expression literals.
+* Regular expression-based sanitisers implemented with `new RegExp(..)` are now detected in more cases.
+* Regular expression related queries now account for unknown flags.

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

@@ -117,6 +117,12 @@ 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 unknown flags, or this is a call to `.replaceAll()`.
+   */
+  predicate maybeGlobal() { this.getRegExp().maybeGlobal() or this.getMethodName() = "replaceAll" }
+
   /**
    * Holds if this call to `replace` replaces `old` with `new`.
    */

javascript/ql/lib/semmle/javascript/dataflow/Nodes.qll

@@ -1685,6 +1685,9 @@ class RegExpCreationNode extends DataFlow::SourceNode {
   /** Holds if the constructed predicate has the `g` flag. */
   predicate isGlobal() { RegExp::isGlobal(this.getFlags()) }
 
+  /** Holds if the constructed predicate has the `g` flag or unknown flags. */
+  predicate maybeGlobal() { RegExp::maybeGlobal(this.tryGetFlags()) }
+
   /** Gets a data flow node referring to this regular expression. */
   private DataFlow::SourceNode getAReference(DataFlow::TypeTracker t) {
     t.start() and

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::RegExpLiteralNode
+      member.maybeGlobal() and member.getArgument(0) instanceof DataFlow::RegExpCreationNode
     )
   }
 

javascript/ql/lib/semmle/javascript/security/dataflow/CleartextLoggingCustomizations.qll

@@ -36,9 +36,12 @@ module CleartextLogging {
    */
   class MaskingReplacer extends Barrier, StringReplaceCall {
     MaskingReplacer() {
-      this.isGlobal() and
+      this.maybeGlobal() and
       exists(this.getRawReplacement().getStringValue()) and
-      any(RegExpDot term).getLiteral() = this.getRegExp().asExpr()
+      exists(DataFlow::RegExpCreationNode regexpObj |
+        this.(StringReplaceCall).getRegExp() = regexpObj and
+        regexpObj.getRoot() = any(RegExpDot term).getRootTerm()
+      )
     }
   }
 

javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutingAssignmentQuery.qll

@@ -46,7 +46,7 @@ class Configuration extends TaintTracking::Configuration {
       // Replacing with "_" is likely to be exploitable
       not replace.getRawReplacement().getStringValue() = "_" and
       (
-        replace.isGlobal()
+        replace.maybeGlobal()
         or
         // Non-global replace with a non-empty string can also prevent __proto__ by
         // inserting a chunk of text that doesn't fit anywhere in __proto__

javascript/ql/lib/semmle/javascript/security/dataflow/RegExpInjectionCustomizations.qll

@@ -76,7 +76,7 @@ module RegExpInjection {
    */
   class MetacharEscapeSanitizer extends Sanitizer, StringReplaceCall {
     MetacharEscapeSanitizer() {
-      this.isGlobal() and
+      this.maybeGlobal() and
       (
         RegExp::alwaysMatchesMetaCharacter(this.getRegExp().getRoot(), ["{", "[", "+"])
         or

javascript/ql/lib/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll

@@ -221,10 +221,10 @@ module TaintedPath {
       this instanceof StringReplaceCall and
       input = this.getReceiver() and
       output = this and
-      not exists(RegExpLiteral literal, RegExpTerm term |
-        this.(StringReplaceCall).getRegExp().asExpr() = literal and
-        this.(StringReplaceCall).isGlobal() and
-        literal.getRoot() = term
+      not exists(DataFlow::RegExpCreationNode regexp, RegExpTerm term |
+        this.(StringReplaceCall).getRegExp() = regexp and
+        this.(StringReplaceCall).maybeGlobal() and
+        regexp.getRoot() = term
       |
         term.getAMatchedString() = "/" or
         term.getAMatchedString() = "." or
@@ -305,9 +305,9 @@ module TaintedPath {
       input = this.getReceiver() and
       output = this and
       this.isGlobal() and
-      exists(RegExpLiteral literal, RegExpTerm term |
-        this.getRegExp().asExpr() = literal and
-        literal.getRoot() = term and
+      exists(DataFlow::RegExpCreationNode regexp, RegExpTerm term |
+        this.getRegExp() = regexp and
+        regexp.getRoot() = term and
         not term.getAMatchedString() = "/"
       |
         term.getAMatchedString() = "." or

javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeShellCommandConstructionCustomizations.qll

@@ -245,7 +245,7 @@ module UnsafeShellCommandConstruction {
   class ReplaceQuotesSanitizer extends Sanitizer, StringReplaceCall {
     ReplaceQuotesSanitizer() {
       this.getAReplacedString() = "'" and
-      this.isGlobal() and
+      this.maybeGlobal() and
       this.getRawReplacement().mayHaveStringValue(["'\\''", ""])
     }
   }

javascript/ql/lib/semmle/javascript/security/dataflow/Xss.qll

@@ -36,7 +36,7 @@ module Shared {
    */
   class MetacharEscapeSanitizer extends Sanitizer, StringReplaceCall {
     MetacharEscapeSanitizer() {
-      this.isGlobal() and
+      this.maybeGlobal() and
       (
         RegExp::alwaysMatchesMetaCharacter(this.getRegExp().getRoot(), ["<", "'", "\""])
         or