JS: Add flow steps through iteration callback

Author: asger-semmle

javascript/ql/src/semmle/javascript/StandardLibrary.qll

@@ -224,3 +224,27 @@ private class PromiseTaintStep extends TaintTracking::AdditionalTaintStep {
     pred = source and succ = this
   }
 }
+
+/**
+ * A flow step propagating the exception thrown from a callback to a method whose name coincides
+ * a built-in Array iteration method, such as `forEach` or `map`.
+ */
+private class IteratorExceptionStep extends DataFlow::MethodCallNode, DataFlow::AdditionalFlowStep {
+  IteratorExceptionStep() {
+    exists(string name | name = getMethodName() |
+      name = "forEach" or
+      name = "each" or
+      name = "map" or
+      name = "filter" or
+      name = "some" or
+      name = "every" or
+      name = "fold" or
+      name = "reduce"
+    )
+  }
+
+  override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+    pred = getAnArgument().(DataFlow::FunctionNode).getExceptionalReturn() and
+    succ = this.getExceptionalReturn()
+  }
+}

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

@@ -141,6 +141,13 @@ class InvokeNode extends DataFlow::SourceNode {
    * likely to be imprecise or incomplete.
    */
   predicate isUncertain() { isImprecise() or isIncomplete() }
+
+  /**
+   * Gets the data flow node representing an exception thrown from this invocation.
+   */
+  DataFlow::ExceptionalInvocationReturnNode getExceptionalReturn() {
+    DataFlow::exceptionalInvocationReturnNode(result, asExpr())
+  }
 }
 
 /** Auxiliary module used to cache a few related predicates together. */
@@ -356,6 +363,13 @@ class FunctionNode extends DataFlow::ValueNode, DataFlow::SourceNode {
    * To get the data flow node for `this` in an arrow function, consider using `getThisBinder().getReceiver()`.
    */
   ThisNode getReceiver() { result.getBinder() = this }
+
+  /**
+   * Gets the data flow node representing an exception thrown from this function.
+   */
+  DataFlow::ExceptionalFunctionReturnNode getExceptionalReturn() {
+    DataFlow::exceptionalFunctionReturnNode(result, astNode)
+  }
 }
 
 /** A data flow node corresponding to an object literal expression. */

javascript/ql/src/semmle/javascript/frameworks/LodashUnderscore.qll

@@ -360,6 +360,46 @@ module LodashUnderscore {
     name = "eachRight" or
     name = "first"
   }
+
+  /**
+   * A data flow step propagating an exception thrown from a callback to a Lodash/Underscore function.
+   */
+  private class ExceptionStep extends DataFlow::CallNode, DataFlow::AdditionalFlowStep {
+    ExceptionStep() {
+      exists(string name |
+        this = member(name).getACall()
+      |
+        // Collection methods
+        name = "countBy" or
+        name = "each" or
+        name = "eachRight" or
+        name = "forEach" or
+        name = "forEachRight" or
+        name = "every" or
+        name = "filter" or
+        name = "groupBy" or
+        name = "orderBy" or
+        name = "partition" or
+        name = "reduce" or
+        name = "reduceRight" or
+        name = "some" or
+        name = "sortBy" or
+
+        // Array methods
+        name = "dropRightWhile" or
+        name = "dropWhile" or
+        name = "sortedIndexBy" or
+        name = "sortedUniqBy" or
+        name = "takeRightWhile" or
+        name = "takeWhile"
+      )
+    }
+
+    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+      pred = getAnArgument().(DataFlow::FunctionNode).getExceptionalReturn() and
+      succ = this.getExceptionalReturn()
+    }
+  }
 }
 
 /**

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

@@ -39,6 +39,8 @@
 | exceptions.js:115:21:115:28 | source() | exceptions.js:121:10:121:10 | e |
 | exceptions.js:144:9:144:16 | source() | exceptions.js:129:10:129:10 | e |
 | 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 |
 | indexOf.js:4:11:4:18 | source() | indexOf.js:9:10:9:10 | x |
 | 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 |

javascript/ql/test/library-tests/TaintTracking/exceptions.js

@@ -144,4 +144,30 @@ function throwSource() {
   throw source();
 }
 
+function throwThoughLibrary(xs) {
+  try {
+    xs.forEach(function() {
+      throw source();
+    })
+  } catch (e) {
+    sink(e); // NOT OK
+  }
+
+  try {
+    _.takeWhile(xs, function() {
+      throw source();
+    })
+  } catch (e) {
+    sink(e); // NOT OK
+  }
+
+  try {
+    window.addEventListener("message", function(e) {
+      throw source();
+    })
+  } catch (e) {
+    sink(e); // OK - doesn't catch exception from event listener
+  }
+}
+
 // semmle-extractor-options: --experimental