JS: Allow MaD flow through properties

javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll

@@ -31,27 +31,16 @@ private class RemoteFlowSourceFromCsv extends RemoteFlowSource {
   override string getSourceType() { result = "Remote flow" }
 }
 
-/**
- * Like `ModelOutput::summaryStep` but with API nodes mapped to data-flow nodes.
- */
-private predicate summaryStepNodes(DataFlow::Node pred, DataFlow::Node succ, string kind) {
-  exists(API::Node predNode, API::Node succNode |
-    Specific::summaryStep(predNode, succNode, kind) and
-    pred = predNode.getARhs() and
-    succ = succNode.getAnImmediateUse()
-  )
-}
-
 /** Data flow steps induced by summary models of kind `value`. */
 private class DataFlowStepFromSummary extends DataFlow::SharedFlowStep {
   override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-    summaryStepNodes(pred, succ, "value")
+    Specific::summaryStep(pred, succ, "value")
   }
 }
 
 /** Taint steps induced by summary models of kind `taint`. */
 private class TaintStepFromSummary extends TaintTracking::SharedTaintStep {
   override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-    summaryStepNodes(pred, succ, "taint")
+    Specific::summaryStep(pred, succ, "taint")
   }
 }

javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll

@@ -132,6 +132,9 @@ module ModelInput {
      * indicates that for each call to `(package, type, path)`, the value referred to by `input`
      * can flow to the value referred to by `output`.
      *
+     * Alternatively, if `input` is the empty string and `output` consists of a single token,
+     * the value referred to by `(package, type, path)` may flow to the value referred to by `output`.
+     *
      * `kind` should be either `value` or `taint`, for value-preserving or taint-preserving steps,
      * respectively.
      */

javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsSpecific.qll

@@ -197,15 +197,26 @@ private API::Node getNodeFromInputOutputPath(API::InvokeNode baseNode, AccessPat
 /**
  * Holds if a CSV summary contributed the step `pred -> succ` of the given `kind`.
  */
-predicate summaryStep(API::Node pred, API::Node succ, string kind) {
+predicate summaryStep(DataFlow::Node pred, DataFlow::Node succ, string kind) {
   exists(
     string package, string type, string path, API::InvokeNode base, AccessPath input,
     AccessPath output
   |
     ModelOutput::relevantSummaryModel(package, type, path, input, output, kind) and
     ModelOutput::resolvedSummaryBase(package, type, path, base) and
-    pred = getNodeFromInputOutputPath(base, input) and
-    succ = getNodeFromInputOutputPath(base, output)
+    pred = getNodeFromInputOutputPath(base, input).getARhs() and
+    succ = getNodeFromInputOutputPath(base, output).getAnImmediateUse()
+  )
+  or
+  exists(
+    string package, string type, string path, API::Node base, AccessPath output,
+    DataFlow::PropRead read
+  |
+    ModelOutput::relevantSummaryModel(package, type, path, "", output, kind) and
+    base = getNodeFromPath(package, type, path) and
+    read = getSuccessorFromNode(base, output).getAnImmediateUse() and
+    pred = read.getBase() and
+    succ = read
   )
 }
 

javascript/ql/test/library-tests/frameworks/data/test.expected

@@ -55,6 +55,7 @@ taintFlow
 | test.js:182:12:182:19 | source() | test.js:182:12:182:19 | source() |
 | test.js:187:31:187:31 | x | test.js:189:10:189:10 | x |
 | test.js:203:32:203:39 | source() | test.js:203:32:203:39 | source() |
+| test.js:209:13:209:20 | source() | test.js:210:8:210:19 | obj.self.foo |
 isSink
 | test.js:54:18:54:25 | source() | test-sink |
 | test.js:55:22:55:29 | source() | test-sink |

javascript/ql/test/library-tests/frameworks/data/test.js

@@ -203,3 +203,9 @@ class OtherClass {
     this.accessorAroundField = source(); // NOT OK
   }
 }
+
+function testFlowThroughProp() {
+  const obj = new testlib.Obj();
+  obj.foo = source();
+  sink(obj.self.foo); // NOT OK
+}

javascript/ql/test/library-tests/frameworks/data/test.ql

@@ -14,6 +14,7 @@ class Steps extends ModelInput::SummaryModelCsv {
         "testlib;;Member[preserveAllButFirstArgument];Argument[1..];ReturnValue;taint",
         "testlib;;Member[preserveAllIfCall].Call;Argument[0..];ReturnValue;taint",
         "testlib;;Member[getSource].ReturnValue.Member[continue];Argument[this];ReturnValue;taint",
+        "testlib;;Member[Obj].Instance;;Member[self];value",
       ]
   }
 }

ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModels.qll

@@ -132,6 +132,9 @@ module ModelInput {
      * indicates that for each call to `(package, type, path)`, the value referred to by `input`
      * can flow to the value referred to by `output`.
      *
+     * Alternatively, if `input` is the empty string and `output` consists of a single token,
+     * the value referred to by `(package, type, path)` may flow to the value referred to by `output`.
+     *
      * `kind` should be either `value` or `taint`, for value-preserving or taint-preserving steps,
      * respectively.
      */