Merge pull request #17261 from asgerf/jss/dynamic-import-step

JS: Port step for dynamic imports

Author: asgerf

javascript/ql/lib/semmle/javascript/Promises.qll

@@ -705,7 +705,6 @@ private module DynamicImportSteps {
    */
   class DynamicImportStep extends LegacyPreCallGraphStep {
     override predicate storeStep(DataFlow::Node pred, DataFlow::SourceNode succ, string prop) {
-      // TODO: this step needs to be ported to dataflow2
       exists(DynamicImportExpr imprt |
         pred = imprt.getImportedModule().getAnExportedValue("default") and
         succ = imprt.flow() and

javascript/ql/lib/semmle/javascript/internal/flow_summaries/AllFlowSummaries.qll

@@ -9,3 +9,4 @@ private import Maps
 private import Promises
 private import Sets
 private import Strings
+private import DynamicImportStep

javascript/ql/lib/semmle/javascript/internal/flow_summaries/DynamicImportStep.qll

@@ -0,0 +1,39 @@
+/**
+ * Contains flow steps to model flow from a module into a dynamic `import()` expression.
+ */
+
+private import javascript
+private import semmle.javascript.dataflow.internal.DataFlowNode
+private import semmle.javascript.dataflow.internal.AdditionalFlowInternal
+private import semmle.javascript.dataflow.internal.DataFlowPrivate
+
+/**
+ * Flow steps for dynamic import expressions.
+ *
+ * The default export of the imported module must be boxed in a promise, so we pass
+ * it through a synthetic node.
+ */
+class DynamicImportStep extends AdditionalFlowInternal {
+  override predicate needsSynthesizedNode(AstNode node, string tag, DataFlowCallable container) {
+    node instanceof DynamicImportExpr and
+    tag = "imported-value" and
+    container.asSourceCallable() = node.getContainer()
+  }
+
+  override predicate jumpStep(DataFlow::Node pred, DataFlow::Node succ) {
+    exists(DynamicImportExpr expr |
+      pred = expr.getImportedModule().getAnExportedValue("default") and
+      succ = getSynthesizedNode(expr, "imported-value")
+    )
+  }
+
+  override predicate storeStep(
+    DataFlow::Node pred, DataFlow::ContentSet contents, DataFlow::Node succ
+  ) {
+    exists(DynamicImportExpr expr |
+      pred = getSynthesizedNode(expr, "imported-value") and
+      contents = DataFlow::ContentSet::promiseValue() and
+      succ = TValueNode(expr)
+    )
+  }
+}

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

@@ -14,6 +14,8 @@ legacyDataFlowDifference
 | constructor-calls.js:4:18:4:25 | source() | constructor-calls.js:44:8:44:19 | f_safe.taint | only flow with NEW data flow library |
 | constructor-calls.js:20:15:20:22 | source() | constructor-calls.js:39:8:39:14 | e.param | only flow with NEW data flow library |
 | exceptions.js:53:14:53:21 | source() | exceptions.js:54:10:54:10 | e | only flow with NEW data flow library |
+| export-taint.js:3:22:3:29 | source() | import-taint.js:7:10:7:25 | mod.object.taint | only flow with OLD data flow library |
+| export-taint.js:3:22:3:29 | source() | import-taint.js:14:14:14:29 | mod.object.taint | only flow with OLD data flow library |
 | getters-and-setters.js:53:21:53:28 | source() | getters-and-setters.js:53:10:53:30 | getX(ne ... rce())) | only flow with NEW data flow library |
 | nested-props.js:14:15:14:22 | source() | nested-props.js:15:10:15:16 | obj.x.y | only flow with NEW data flow library |
 | nested-props.js:27:18:27:25 | source() | nested-props.js:28:10:28:14 | obj.x | only flow with NEW data flow library |
@@ -165,6 +167,8 @@ flow
 | exceptions.js:144:9:144:16 | source() | exceptions.js:132:8:132:27 | returnThrownSource() |
 | exceptions.js:150:13:150:20 | source() | exceptions.js:153:10:153:10 | e |
 | exceptions.js:158:13:158:20 | source() | exceptions.js:161:10:161:10 | e |
+| export-taint.js:2:12:2:19 | source() | import-taint.js:6:10:6:18 | mod.taint |
+| export-taint.js:2:12:2:19 | source() | import-taint.js:13:14:13:22 | mod.taint |
 | factory-function.js:21:13:21:20 | source() | factory-function.js:7:10:7:12 | obj |
 | factory-function.js:22:13:22:20 | source() | factory-function.js:7:10:7:12 | obj |
 | factory-function.js:26:7:26:14 | source() | factory-function.js:16:14:16:16 | obj |

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

@@ -15,6 +15,8 @@ legacyDataFlowDifference
 | constructor-calls.js:4:18:4:25 | source() | constructor-calls.js:44:8:44:19 | f_safe.taint | only flow with NEW data flow library |
 | constructor-calls.js:20:15:20:22 | source() | constructor-calls.js:39:8:39:14 | e.param | only flow with NEW data flow library |
 | exceptions.js:53:14:53:21 | source() | exceptions.js:54:10:54:10 | e | only flow with NEW data flow library |
+| export-taint.js:3:22:3:29 | source() | import-taint.js:7:10:7:25 | mod.object.taint | only flow with OLD data flow library |
+| export-taint.js:3:22:3:29 | source() | import-taint.js:14:14:14:29 | mod.object.taint | only flow with OLD data flow library |
 | getters-and-setters.js:53:21:53:28 | source() | getters-and-setters.js:53:10:53:30 | getX(ne ... rce())) | only flow with NEW data flow library |
 | nested-props.js:14:15:14:22 | source() | nested-props.js:15:10:15:16 | obj.x.y | only flow with NEW data flow library |
 | nested-props.js:27:18:27:25 | source() | nested-props.js:28:10:28:14 | obj.x | only flow with NEW data flow library |
@@ -115,6 +117,8 @@ flow
 | exceptions.js:144:9:144:16 | source() | exceptions.js:132:8:132:27 | returnThrownSource() |
 | exceptions.js:150:13:150:20 | source() | exceptions.js:153:10:153:10 | e |
 | exceptions.js:158:13:158:20 | source() | exceptions.js:161:10:161:10 | e |
+| export-taint.js:2:12:2:19 | source() | import-taint.js:6:10:6:18 | mod.taint |
+| export-taint.js:2:12:2:19 | source() | import-taint.js:13:14:13:22 | mod.taint |
 | factory-function.js:21:13:21:20 | source() | factory-function.js:7:10:7:12 | obj |
 | factory-function.js:22:13:22:20 | source() | factory-function.js:7:10:7:12 | obj |
 | factory-function.js:26:7:26:14 | source() | factory-function.js:16:14:16:16 | obj |

javascript/ql/test/library-tests/TaintTracking/export-taint.js

@@ -0,0 +1,4 @@
+export default {
+    taint: source(),
+    object: { taint: source() }
+};

javascript/ql/test/library-tests/TaintTracking/import-taint.js

@@ -0,0 +1,16 @@
+import 'dummy';
+
+async function test1() {
+    let mod = await import("./export-taint");
+    sink(mod); // OK
+    sink(mod.taint); // NOT OK
+    sink(mod.object.taint); // NOT OK [INCONSISTENCY] - blocked by access path limit
+}
+
+function test2() {
+    import("./export-taint").then(mod => {
+        sink(mod); // OK
+        sink(mod.taint); // NOT OK
+        sink(mod.object.taint); // NOT OK [INCONSISTENCY] - blocked by access path limit
+    });
+}