JS: address comments and support TrackedNode

Author: asger-semmle

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

@@ -607,7 +607,7 @@ private predicate storeStep(
   summary = PathSummary::level()
   or
   exists(Function f, DataFlow::Node mid |
-    // `f` stores its parameter `pred` in property `prop` of a value that it returns,
+    // `f` stores its parameter `pred` in property `prop` of a value that flows back to the caller,
     // and `succ` is an invocation of `f`
     reachableFromInput(f, succ, pred, mid, cfg, summary) and
     (
@@ -619,21 +619,6 @@ private predicate storeStep(
   )
 }
 
-/**
- * Holds if `f` may return `base`, which has a write of property `prop` with right-hand side `rhs`.
- */
-predicate returnedPropWrite(Function f, DataFlow::SourceNode base, string prop, DataFlow::Node rhs) {
-  base.hasPropertyWrite(prop, rhs) and
-  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/TrackedNodes.qll

@@ -167,12 +167,16 @@ private module NodeTracking {
     basicStoreStep(pred, succ, prop) and
     summary = PathSummary::level()
     or
-    exists(Function f, DataFlow::Node mid, DataFlow::SourceNode base |
-      // `f` stores its parameter `pred` in property `prop` of a value that it returns,
+    exists(Function f, DataFlow::Node mid |
+      // `f` stores its parameter `pred` in property `prop` of a value that flows back to the caller,
       // and `succ` is an invocation of `f`
       reachableFromInput(f, succ, pred, mid, summary) and
-      base.hasPropertyWrite(prop, mid) and
-      base.flowsToExpr(f.getAReturnedExpr())
+      (
+        returnedPropWrite(f, _, prop, mid)
+        or
+        succ instanceof DataFlow::NewNode and
+        receiverPropWrite(f, prop, mid)
+      )
     )
   }
 

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

@@ -115,7 +115,7 @@ predicate callStep(DataFlow::Node pred, DataFlow::Node succ) {
 
 /**
  * Holds if there is a flow step from `pred` to `succ` through returning
- * from a function call.
+ * from a function call or the receiver flowing out of a constructor call.
  */
 predicate returnStep(DataFlow::Node pred, DataFlow::Node succ) {
   exists(Function f | calls(succ, f) |
@@ -266,6 +266,21 @@ predicate callback(DataFlow::Node arg, DataFlow::SourceNode cb) {
   )
 }
 
+/**
+ * Holds if `f` may return `base`, which has a write of property `prop` with right-hand side `rhs`.
+ */
+predicate returnedPropWrite(Function f, DataFlow::SourceNode base, string prop, DataFlow::Node rhs) {
+  base.hasPropertyWrite(prop, rhs) and
+  base.flowsToExpr(f.getAReturnedExpr())
+}
+
+/**
+ * Holds if `f` may assign `rhs` to `this.prop`.
+ */
+predicate receiverPropWrite(Function f, string prop, DataFlow::Node rhs) {
+  DataFlow::thisNode(f).hasPropertyWrite(prop, rhs)
+}
+
 /**
  * A utility class that is equivalent to `boolean` but does not require type joining.
  */

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

@@ -0,0 +1,12 @@
+import * as dummy from 'dummy';
+
+class C {
+  constructor() {
+    this.field = new Object();
+  }
+}
+
+function test() {
+  let x = new C().field;
+  foo(x);
+}