support interprocedural flow with custom load/store steps

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

@@ -466,22 +466,22 @@ abstract class AdditionalFlowStep extends DataFlow::Node {
   }
 
   /**
-   * Holds if the `pred` should be stored in the object `succ` under the property `prop`. 
+   * Holds if the `pred` should be stored in the object `succ` under the property `prop`.
    */
   cached
   predicate store(DataFlow::Node pred, DataFlow::Node succ, string prop) { none() }
 
   /**
-   * Holds if the property `prop` of the object `pred` should be loaded into `succ`. 
+   * Holds if the property `prop` of the object `pred` should be loaded into `succ`.
    */
   cached
   predicate load(DataFlow::Node pred, DataFlow::Node succ, string prop) { none() }
 
   /**
-   * Holds if the property `prop` should be copied from the object `pred` to the object `succ`. 
+   * Holds if the property `prop` should be copied from the object `pred` to the object `succ`.
    */
   cached
-  predicate copyProperty(DataFlow::Node pred, DataFlow::Node succ, string prop) { none() } 
+  predicate copyProperty(DataFlow::Node pred, DataFlow::Node succ, string prop) { none() }
 }
 
 /**
@@ -584,7 +584,7 @@ private predicate exploratoryFlowStep(
   basicFlowStep(pred, succ, _, cfg) or
   basicStoreStep(pred, succ, _) or
   basicLoadStep(pred, succ, _) or
-  
+
   any(AdditionalFlowStep s).store(pred, succ, _) or
   cfg.isAdditionalStoreStep(pred, succ, _) or
   any(AdditionalFlowStep s).load(pred, succ, _) or
@@ -765,6 +765,16 @@ private predicate storeStep(
     (
       returnedPropWrite(f, _, prop, mid)
       or
+      exists(DataFlow::SourceNode base | 
+        (
+          any(AdditionalFlowStep step).store(mid, _, prop)
+          or
+          cfg.isAdditionalStoreStep(mid, _, prop)
+        ) 
+        and
+        base.flowsToExpr(f.getAReturnedExpr())
+      )
+      or
       succ instanceof DataFlow::NewNode and
       receiverPropWrite(f, prop, mid)
     )
@@ -775,12 +785,18 @@ private predicate storeStep(
  * Holds if `f` may `read` property `prop` of parameter `parm`.
  */
 private predicate parameterPropRead(
-  Function f, DataFlow::Node invk, DataFlow::Node arg, string prop, DataFlow::PropRead read,
+  Function f, DataFlow::Node invk, DataFlow::Node arg, string prop, DataFlow::Node read,
   DataFlow::Configuration cfg
 ) {
-  exists(DataFlow::SourceNode parm |
+  exists(DataFlow::Node parm |
     callInputStep(f, invk, arg, parm, cfg) and
-    read = parm.getAPropertyRead(prop)
+    (
+      read = parm.(DataFlow::SourceNode).getAPropertyRead(prop)
+      or
+      any(AdditionalFlowStep step).load(parm, read, prop)
+      or
+      cfg.isAdditionalLoadStep(parm, read, prop)
+    )
   )
 }
 
@@ -819,7 +835,7 @@ private predicate loadStep(
   cfg.isAdditionalLoadStep(pred, succ, prop) and
   summary = PathSummary::level()
   or
-  exists(Function f, DataFlow::PropRead read |
+  exists(Function f, DataFlow::Node read |
     parameterPropRead(f, succ, pred, prop, read, cfg) and
     reachesReturn(f, read, cfg, summary)
   )
@@ -866,17 +882,17 @@ private predicate flowThroughProperty(
 
 /**
  * Holds if the property `prop` is copied from `fromNode` to `toNode` using at least 1 step.
- * 
- * The recursion of this predicate has been unfolded once compared to a naive implementation in order to avoid having no constraint on `prop`. 
- * Therefore a caller of this predicate should also test whether the `toNode` and `fromNode` are equal. 
+ *
+ * The recursion of this predicate has been unfolded once compared to a naive implementation in order to avoid having no constraint on `prop`.
+ * Therefore a caller of this predicate should also test whether the `toNode` and `fromNode` are equal.
  */
 private predicate existsCopyProperty(DataFlow::Node fromNode, DataFlow::Node toNode, string prop) {
-  exists(DataFlow::AdditionalFlowStep step, DataFlow::Node mid | 
+  exists(DataFlow::AdditionalFlowStep step, DataFlow::Node mid |
     step.copyProperty(fromNode, mid, prop) and
     (
       existsCopyProperty(mid, toNode, prop)
       or
-      mid = toNode  
+      mid = toNode
     )
   )
 }

javascript/ql/test/library-tests/CustomLoadStoreSteps/test.expected

@@ -0,0 +1 @@
+| tst.js:4:15:4:22 | "source" | tst.js:9:7:9:24 | readTaint(tainted) |

javascript/ql/test/library-tests/CustomLoadStoreSteps/test.ql

@@ -0,0 +1,22 @@
+import javascript
+
+class Configuration extends TaintTracking::Configuration {
+  Configuration() { this = "PromiseFlowTestingConfig" }
+
+  override predicate isSource(DataFlow::Node source) {
+    source.getEnclosingExpr().getStringValue() = "source"
+  }
+
+  override predicate isSink(DataFlow::Node sink) {
+    any(DataFlow::InvokeNode call | call.getCalleeName() = "sink").getAnArgument() = sink
+  }
+
+  // When the source code states that "foo" is being read, "bar" is additionally being read.
+  override predicate isAdditionalLoadStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
+    pred.(DataFlow::SourceNode).getAPropertyRead("foo") = succ and prop = "bar"
+  }
+}
+
+from DataFlow::Node pred, DataFlow::Node succ, Configuration cfg
+where cfg.hasFlow(pred, succ)
+select pred, succ

javascript/ql/test/library-tests/CustomLoadStoreSteps/tst.js

@@ -0,0 +1,10 @@
+// When the source code states that "foo" is being read, "bar" is additionally being read.
+
+(function () {
+	var source = "source";
+	var tainted = { bar: source };
+	function readTaint(x) {
+		return x.foo;
+	}
+	sink(readTaint(tainted));
+})();

javascript/ql/test/library-tests/Promises/flow.js

@@ -13,7 +13,7 @@
 	var p4 = new Promise((resolve, reject) => reject(source));
 	try {
 		var foo = await p4;
-	} catch(e) {
+	} catch (e) {
 		sink(e); // NOT OK!
 	}
 
@@ -31,19 +31,24 @@
 
 	new Promise((resolve, reject) => reject(source)).catch(x => sink(x)); // NOT OK!
 
-	Promise.resolve(source).catch(() => {}).then(a => sink(a)); // NOT OK!
+	Promise.resolve(source).catch(() => { }).then(a => sink(a)); // NOT OK!
 
 	var p5 = Promise.resolve(source);
-	var p6 = p5.catch(() => {});
+	var p6 = p5.catch(() => { });
 	var p7 = p6.then(a => sink(a)); // NOT OK!
 
-	new Promise((resolve, reject) => reject(source)).then(() => {}).catch(x => sink(x)); // NOT OK!
+	new Promise((resolve, reject) => reject(source)).then(() => { }).catch(x => sink(x)); // NOT OK!
 
-	new Promise((resolve, reject) => reject(source)).then(() => {}, () => {}).catch(x => sink(x)); // OK!
+	new Promise((resolve, reject) => reject(source)).then(() => { }, () => { }).catch(x => sink(x)); // OK!
 
-	Promise.resolve(source).catch(() => {}).catch(() => {}).catch(() => {}).then(a => sink(a)); // NOT OK!
+	Promise.resolve(source).catch(() => { }).catch(() => { }).catch(() => { }).then(a => sink(a)); // NOT OK!
 
-	Promise.resolve(source).finally(() => {}).then(a => sink(a)); // NOT OK!
+	Promise.resolve(source).finally(() => { }).then(a => sink(a)); // NOT OK!
 
-	new Promise(() => {throw source}).catch(x => sink(x)); // NOT OK!
+	new Promise(() => { throw source }).catch(x => sink(x)); // NOT OK!
+
+	function createPromise(src) {
+		return Promise.resolve(src);
+	}
+	createPromise(source).then(v => sink(v)); // NOT OK!
 })();

javascript/ql/test/library-tests/Promises/interflow.js

@@ -0,0 +1,20 @@
+(function () {
+  function getSource() {
+    var source = "source"; // step 1
+    return source; // step 2
+  }
+  loadScript(getSource()) // step 3
+    .then(function () { })
+    .then(function () { })
+    .catch(handleError);
+  function loadScript(src) { // step 4 (is summarized)
+    return new Promise(function (resolve, reject) {
+      setTimeout(function (error) {
+        reject(new Error('Blah: ' + src)); // step 5
+      }, 1000);
+    });
+  }
+  function handleError(error) { // step 6
+    sink(error); // step 7
+  }
+})();

javascript/ql/test/library-tests/Promises/tests.expected

@@ -8,14 +8,15 @@ test_ResolvedPromiseDefinition
 | flow.js:36:11:36:33 | Promise ... source) | flow.js:36:27:36:32 | source |
 | flow.js:44:2:44:24 | Promise ... source) | flow.js:44:18:44:23 | source |
 | flow.js:46:2:46:24 | Promise ... source) | flow.js:46:18:46:23 | source |
+| flow.js:51:10:51:29 | Promise.resolve(src) | flow.js:51:26:51:28 | src |
 | promises.js:53:19:53:41 | Promise ... source) | promises.js:53:35:53:40 | source |
 | promises.js:62:19:62:41 | Promise ... source) | promises.js:62:35:62:40 | source |
 | promises.js:71:5:71:27 | Promise ... source) | promises.js:71:21:71:26 | source |
 test_PromiseDefinition_getARejectHandler
 | flow.js:26:2:26:49 | new Pro ... ource)) | flow.js:26:69:26:80 | y => sink(y) |
 | flow.js:32:2:32:49 | new Pro ... ource)) | flow.js:32:57:32:68 | x => sink(x) |
-| flow.js:42:2:42:49 | new Pro ... ource)) | flow.js:42:66:42:73 | () => {} |
-| flow.js:48:2:48:34 | new Pro ... ource}) | flow.js:48:42:48:53 | x => sink(x) |
+| flow.js:42:2:42:49 | new Pro ... ource)) | flow.js:42:67:42:75 | () => { } |
+| flow.js:48:2:48:36 | new Pro ... urce }) | flow.js:48:44:48:55 | x => sink(x) |
 | promises.js:10:18:17:4 | new Pro ... );\\n  }) | promises.js:20:6:22:3 | (v) =>  ...  v;\\n  } |
 | promises.js:10:18:17:4 | new Pro ... );\\n  }) | promises.js:23:18:25:3 | (v) =>  ...  v;\\n  } |
 | promises.js:10:18:17:4 | new Pro ... );\\n  }) | promises.js:26:20:28:3 | (v) =>  ...  v;\\n  } |
@@ -28,7 +29,8 @@ test_PromiseDefinition_getExecutor
 | flow.js:32:2:32:49 | new Pro ... ource)) | flow.js:32:14:32:48 | (resolv ... source) |
 | flow.js:40:2:40:49 | new Pro ... ource)) | flow.js:40:14:40:48 | (resolv ... source) |
 | flow.js:42:2:42:49 | new Pro ... ource)) | flow.js:42:14:42:48 | (resolv ... source) |
-| flow.js:48:2:48:34 | new Pro ... ource}) | flow.js:48:14:48:33 | () => {throw source} |
+| flow.js:48:2:48:36 | new Pro ... urce }) | flow.js:48:14:48:35 | () => { ... ource } |
+| interflow.js:11:12:15:6 | new Pro ... \\n    }) | interflow.js:11:24:15:5 | functio ... ;\\n    } |
 | promises.js:3:17:5:4 | new Pro ... );\\n  }) | promises.js:3:29:5:3 | functio ... e);\\n  } |
 | promises.js:10:18:17:4 | new Pro ... );\\n  }) | promises.js:10:30:17:3 | (res, r ... e);\\n  } |
 | promises.js:33:19:35:6 | new Pro ... \\n    }) | promises.js:33:31:35:5 | functio ... ;\\n    } |
@@ -44,16 +46,17 @@ test_PromiseDefinition
 | flow.js:32:2:32:49 | new Pro ... ource)) |
 | flow.js:40:2:40:49 | new Pro ... ource)) |
 | flow.js:42:2:42:49 | new Pro ... ource)) |
-| flow.js:48:2:48:34 | new Pro ... ource}) |
+| flow.js:48:2:48:36 | new Pro ... urce }) |
+| interflow.js:11:12:15:6 | new Pro ... \\n    }) |
 | promises.js:3:17:5:4 | new Pro ... );\\n  }) |
 | promises.js:10:18:17:4 | new Pro ... );\\n  }) |
 | promises.js:33:19:35:6 | new Pro ... \\n    }) |
 | promises.js:43:19:45:6 | Q.Promi ... \\n    }) |
 test_PromiseDefinition_getAResolveHandler
 | flow.js:24:2:24:49 | new Pro ... ource)) | flow.js:24:56:24:67 | x => sink(x) |
 | flow.js:26:2:26:49 | new Pro ... ource)) | flow.js:26:56:26:66 | x => foo(x) |
-| flow.js:40:2:40:49 | new Pro ... ource)) | flow.js:40:56:40:63 | () => {} |
-| flow.js:42:2:42:49 | new Pro ... ource)) | flow.js:42:56:42:63 | () => {} |
+| flow.js:40:2:40:49 | new Pro ... ource)) | flow.js:40:56:40:64 | () => { } |
+| flow.js:42:2:42:49 | new Pro ... ource)) | flow.js:42:56:42:64 | () => { } |
 | promises.js:3:17:5:4 | new Pro ... );\\n  }) | promises.js:6:16:8:3 | functio ... al;\\n  } |
 | promises.js:10:18:17:4 | new Pro ... );\\n  }) | promises.js:18:17:20:3 | (v) =>  ...  v;\\n  } |
 | promises.js:10:18:17:4 | new Pro ... );\\n  }) | promises.js:26:20:28:3 | (v) =>  ...  v;\\n  } |
@@ -68,6 +71,7 @@ test_PromiseDefinition_getRejectParameter
 | flow.js:32:2:32:49 | new Pro ... ource)) | flow.js:32:24:32:29 | reject |
 | flow.js:40:2:40:49 | new Pro ... ource)) | flow.js:40:24:40:29 | reject |
 | flow.js:42:2:42:49 | new Pro ... ource)) | flow.js:42:24:42:29 | reject |
+| interflow.js:11:12:15:6 | new Pro ... \\n    }) | interflow.js:11:43:11:48 | reject |
 | promises.js:3:17:5:4 | new Pro ... );\\n  }) | promises.js:3:48:3:53 | reject |
 | promises.js:10:18:17:4 | new Pro ... );\\n  }) | promises.js:10:36:10:38 | rej |
 | promises.js:33:19:35:6 | new Pro ... \\n    }) | promises.js:33:50:33:55 | reject |
@@ -81,13 +85,14 @@ test_PromiseDefinition_getResolveParameter
 | flow.js:32:2:32:49 | new Pro ... ource)) | flow.js:32:15:32:21 | resolve |
 | flow.js:40:2:40:49 | new Pro ... ource)) | flow.js:40:15:40:21 | resolve |
 | flow.js:42:2:42:49 | new Pro ... ource)) | flow.js:42:15:42:21 | resolve |
+| interflow.js:11:12:15:6 | new Pro ... \\n    }) | interflow.js:11:34:11:40 | resolve |
 | promises.js:3:17:5:4 | new Pro ... );\\n  }) | promises.js:3:39:3:45 | resolve |
 | promises.js:10:18:17:4 | new Pro ... );\\n  }) | promises.js:10:31:10:33 | res |
 | promises.js:33:19:35:6 | new Pro ... \\n    }) | promises.js:33:41:33:47 | resolve |
 | promises.js:43:19:45:6 | Q.Promi ... \\n    }) | promises.js:43:39:43:45 | resolve |
 test_PromiseDefinition_getACatchHandler
 | flow.js:32:2:32:49 | new Pro ... ource)) | flow.js:32:57:32:68 | x => sink(x) |
-| flow.js:48:2:48:34 | new Pro ... ource}) | flow.js:48:42:48:53 | x => sink(x) |
+| flow.js:48:2:48:36 | new Pro ... urce }) | flow.js:48:44:48:55 | x => sink(x) |
 | promises.js:10:18:17:4 | new Pro ... );\\n  }) | promises.js:23:18:25:3 | (v) =>  ...  v;\\n  } |
 flow
 | flow.js:2:15:2:22 | "source" | flow.js:5:7:5:14 | await p1 |
@@ -97,9 +102,11 @@ flow
 | flow.js:2:15:2:22 | "source" | flow.js:26:79:26:79 | y |
 | flow.js:2:15:2:22 | "source" | flow.js:28:58:28:58 | z |
 | flow.js:2:15:2:22 | "source" | flow.js:32:67:32:67 | x |
-| flow.js:2:15:2:22 | "source" | flow.js:34:57:34:57 | a |
+| flow.js:2:15:2:22 | "source" | flow.js:34:58:34:58 | a |
 | flow.js:2:15:2:22 | "source" | flow.js:38:29:38:29 | a |
-| flow.js:2:15:2:22 | "source" | flow.js:40:82:40:82 | x |
-| flow.js:2:15:2:22 | "source" | flow.js:44:89:44:89 | a |
-| flow.js:2:15:2:22 | "source" | flow.js:46:59:46:59 | a |
-| flow.js:2:15:2:22 | "source" | flow.js:48:52:48:52 | x |
+| flow.js:2:15:2:22 | "source" | flow.js:40:83:40:83 | x |
+| flow.js:2:15:2:22 | "source" | flow.js:44:92:44:92 | a |
+| flow.js:2:15:2:22 | "source" | flow.js:46:60:46:60 | a |
+| flow.js:2:15:2:22 | "source" | flow.js:48:54:48:54 | x |
+| flow.js:2:15:2:22 | "source" | flow.js:53:39:53:39 | v |
+| interflow.js:3:18:3:25 | "source" | interflow.js:18:10:18:14 | error |