JS: support flow out of "this" in constructor call

Author: asger-semmle

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

@@ -606,11 +606,16 @@ private predicate storeStep(
   basicStoreStep(pred, succ, prop) and
   summary = PathSummary::level()
   or
-  exists(Function f, DataFlow::Node mid, DataFlow::Node base |
+  exists(Function f, DataFlow::Node mid |
     // `f` stores its parameter `pred` in property `prop` of a value that it returns,
     // and `succ` is an invocation of `f`
     reachableFromInput(f, succ, pred, mid, cfg, summary) and
-    returnedPropWrite(f, base, prop, mid)
+    (
+      returnedPropWrite(f, _, prop, mid)
+      or
+      succ instanceof DataFlow::NewNode and
+      receiverPropWrite(f, prop, mid)
+    )
   )
 }
 
@@ -622,6 +627,13 @@ predicate returnedPropWrite(Function f, DataFlow::SourceNode base, string prop,
   base.flowsToExpr(f.getAReturnedExpr())
 }
 
+/**
+ * Holds if `f` may return `base`, which has a write of property `prop` with right-hand side `rhs`.
+ */
+predicate receiverPropWrite(Function f, string prop, DataFlow::Node rhs) {
+  DataFlow::thisNode(f).hasPropertyWrite(prop, rhs)
+}
+
 /**
  * Holds if `rhs` is the right-hand side of a write to property `prop`, and `nd` is reachable
  * from the base of that write under configuration `cfg` (possibly through callees) along a

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

@@ -802,6 +802,13 @@ module DataFlow {
    */
   predicate thisNode(DataFlow::Node node, StmtContainer container) { node = TThisNode(container) }
 
+  /**
+   * Gets the node representing the receiver of the given function, or `this` in the given top-level.
+   *
+   * Has no result if `container` is an arrow function.
+   */
+  DataFlow::ThisNode thisNode(StmtContainer container) { result = TThisNode(container) }
+
   /**
    * A classification of flows that are not modeled, or only modeled incompletely, by
    * `DataFlowNode`:

javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll

@@ -118,9 +118,11 @@ predicate callStep(DataFlow::Node pred, DataFlow::Node succ) {
  * from a function call.
  */
 predicate returnStep(DataFlow::Node pred, DataFlow::Node succ) {
-  exists(Function f |
-    returnExpr(f, pred, _) and
-    calls(succ, f)
+  exists(Function f | calls(succ, f) |
+    returnExpr(f, pred, _)
+    or
+    succ instanceof DataFlow::NewNode and
+    DataFlow::thisNode(pred, f)
   )
 }
 

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

@@ -1,4 +1,10 @@
 | advanced-callgraph.js:2:13:2:20 | source() | advanced-callgraph.js:6:22:6:22 | v |
+| constructor-calls.js:4:18:4:25 | source() | constructor-calls.js:18:8:18:14 | c.taint |
+| constructor-calls.js:4:18:4:25 | source() | constructor-calls.js:22:8:22:19 | c_safe.taint |
+| constructor-calls.js:10:16:10:23 | source() | constructor-calls.js:26:8:26:14 | d.taint |
+| constructor-calls.js:10:16:10:23 | source() | constructor-calls.js:30:8:30:19 | d_safe.taint |
+| constructor-calls.js:14:15:14:22 | source() | constructor-calls.js:17:8:17:14 | c.param |
+| constructor-calls.js:14:15:14:22 | source() | constructor-calls.js:25:8:25:14 | d.param |
 | partialCalls.js:4:17:4:24 | source() | partialCalls.js:17:14:17:14 | x |
 | partialCalls.js:4:17:4:24 | source() | partialCalls.js:20:14:20:14 | y |
 | partialCalls.js:4:17:4:24 | source() | partialCalls.js:30:14:30:20 | x.value |

javascript/ql/test/library-tests/TaintTracking/constructor-calls.js

@@ -0,0 +1,31 @@
+class EcmaClass {
+  constructor(param) {
+    this.param = param;
+    this.taint = source();
+  }
+}
+
+function JsClass(param) {
+  this.param = param;
+  this.taint = source();
+}
+
+function test() {
+  let taint = source();
+
+  let c = new EcmaClass(taint);
+  sink(c.param); // NOT OK
+  sink(c.taint); // NOT OK
+  
+  let c_safe = new EcmaClass("safe");
+  sink(c_safe.param); // OK
+  sink(c_safe.taint); // NOT OK
+  
+  let d = new JsClass(taint);
+  sink(d.param); // NOT OK
+  sink(d.taint); // NOT OK
+  
+  let d_safe = new JsClass("safe");
+  sink(d_safe.param); // OK
+  sink(d_safe.taint); // NOT OK
+}