JS: Propagate into RegExp.$x

Author: asgerf

javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll

@@ -705,6 +705,68 @@ module TaintTracking {
     }
   }
 
+  private module RegExpCaptureSteps {
+    /** Gets a reference to a string derived from the most recent RegExp match, such as `RegExp.$1` */
+    private DataFlow::PropRead getAStaticCaptureRef() {
+      result =
+        DataFlow::globalVarRef("RegExp")
+            .getAPropertyRead(["$" + [1 .. 9], "input", "lastMatch", "leftContext", "rightContext",
+                  "$&", "$^", "$`"])
+    }
+
+    /**
+     * Gets a control-flow node where `input` is used in a RegExp match.
+     */
+    private ControlFlowNode getACaptureSetter(DataFlow::Node input) {
+      exists(DataFlow::MethodCallNode call | result = call.asExpr() |
+        call.getMethodName() = ["search", "replace", "match"] and input = call.getReceiver()
+        or
+        call.getMethodName() = ["test", "exec"] and input = call.getArgument(0)
+      )
+    }
+
+    /**
+     * Gets a control-flow node that can locally reach the given static capture reference
+     * without passing through a capture setter.
+     *
+     * This is essentially an intraprocedural def-use analysis that ignores potential
+     * side effects from calls.
+     */
+    private ControlFlowNode getANodeReachingCaptureRef(DataFlow::PropRead read) {
+      result = read.asExpr() and
+      read = getAStaticCaptureRef()
+      or
+      exists(ControlFlowNode mid |
+        mid = getANodeReachingCaptureRef(read) and
+        not mid = getACaptureSetter(_) and
+        result = mid.getAPredecessor()
+      )
+    }
+
+    /**
+     * Holds if there is a step `pred -> succ` from the input of a RegExp match to
+     * a static property of `RegExp` defined.
+     */
+    private predicate staticRegExpCaptureStep(DataFlow::Node pred, DataFlow::Node succ) {
+      getACaptureSetter(pred) = getANodeReachingCaptureRef(succ)
+      or
+      exists(DataFlow::MethodCallNode replace |
+        replace.getMethodName() = "replace" and
+        getANodeReachingCaptureRef(succ) = replace.getCallback(1).getFunction().getEntry() and
+        pred = replace.getReceiver()
+      )
+    }
+
+    private class StaticRegExpCaptureStep extends AdditionalTaintStep {
+      StaticRegExpCaptureStep() { staticRegExpCaptureStep(this, _) }
+
+      override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+        pred = this and
+        staticRegExpCaptureStep(this, succ)
+      }
+    }
+  }
+
   /**
    * A conditional checking a tainted string against a regular expression, which is
    * considered to be a sanitizer for all configurations.

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

@@ -103,6 +103,14 @@ typeInferenceMismatch
 | spread.js:2:15:2:22 | source() | spread.js:5:8:5:43 | { f: 'h ... orld' } |
 | spread.js:2:15:2:22 | source() | spread.js:7:8:7:19 | [ ...taint ] |
 | spread.js:2:15:2:22 | source() | spread.js:8:8:8:28 | [ 1, 2, ... nt, 3 ] |
+| static-capture-groups.js:2:17:2:24 | source() | static-capture-groups.js:5:14:5:22 | RegExp.$1 |
+| static-capture-groups.js:2:17:2:24 | source() | static-capture-groups.js:15:14:15:22 | RegExp.$1 |
+| static-capture-groups.js:2:17:2:24 | source() | static-capture-groups.js:17:14:17:22 | RegExp.$1 |
+| static-capture-groups.js:2:17:2:24 | source() | static-capture-groups.js:21:14:21:22 | RegExp.$1 |
+| static-capture-groups.js:2:17:2:24 | source() | static-capture-groups.js:24:14:24:22 | RegExp.$1 |
+| static-capture-groups.js:2:17:2:24 | source() | static-capture-groups.js:27:14:27:22 | RegExp.$1 |
+| static-capture-groups.js:32:17:32:24 | source() | static-capture-groups.js:38:10:38:18 | RegExp.$1 |
+| static-capture-groups.js:42:12:42:19 | source() | static-capture-groups.js:43:14:43:22 | RegExp.$1 |
 | thisAssignments.js:4:17:4:24 | source() | thisAssignments.js:5:10:5:18 | obj.field |
 | thisAssignments.js:7:19:7:26 | source() | thisAssignments.js:8:10:8:20 | this.field2 |
 | tst.js:2:13:2:20 | source() | tst.js:4:10:4:10 | x |

javascript/ql/test/library-tests/TaintTracking/static-capture-groups.js

@@ -8,13 +8,13 @@ function test(x) {
     if (/Foo (.*)/.exec(x)) {
         sink(RegExp.$1); // OK
     } else {
-        sink(RegExp.$1); // NOT OK - previous capture group remains
+        sink(RegExp.$1); // NOT OK [INCONSISTENCY] - previous capture group remains
     }
 
     if (/Hello ([a-zA-Z]+)/.exec(taint)) {
-        sink(RegExp.$1); // OK - capture group is sanitized
+        sink(RegExp.$1); // OK [INCONSISTENCY] - capture group is sanitized
     } else {
-        sink(RegExp.$1); // NOT OK - original capture group possibly remains
+        sink(RegExp.$1); // NOT OK [found but for the wrong reason] - original capture group possibly remains
     }
 
     if (/Hello (.*)/.exec(taint) && something()) {