JS: Port exception steps to a universal summary

Author: asgerf

javascript/ql/lib/semmle/javascript/StandardLibrary.qll

@@ -69,7 +69,7 @@ private class ArrayIterationCallbackAsPartialInvoke extends DataFlow::PartialInv
  * 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::SharedFlowStep {
+private class IteratorExceptionStep extends DataFlow::LegacyFlowStep {
   override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
     exists(DataFlow::MethodCallNode call |
       call.getMethodName() = ["forEach", "each", "map", "filter", "some", "every", "fold", "reduce"] and

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

@@ -1,6 +1,7 @@
 private import AmbiguousCoreMethods
 private import Arrays
 private import AsyncAwait
+private import ExceptionFlow
 private import ForOfLoops
 private import Generators
 private import Iterators

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

@@ -0,0 +1,32 @@
+/**
+ * Contains a summary for propagating exceptions out of callbacks
+ */
+
+private import javascript
+private import FlowSummaryUtil
+private import semmle.javascript.dataflow.internal.AdditionalFlowInternal
+private import semmle.javascript.dataflow.FlowSummary
+private import semmle.javascript.internal.flow_summaries.Promises
+
+/**
+ * Summary that propagates exceptions out of callbacks back to the caller.
+ */
+private class ExceptionFlowSummary extends SummarizedCallable {
+  ExceptionFlowSummary() { this = "Exception propagator" }
+
+  override DataFlow::CallNode getACall() {
+    not exists(result.getACallee()) and
+    // Avoid a few common cases where the exception should not propagate back
+    not result.getCalleeName() =
+      ["then", "catch", "finally", "addEventListener", EventEmitter::on()] and
+    not result = promiseConstructorRef().getAnInvocation() and
+    // Restrict to cases where a callback is known to flow in, as lambda flow in DataFlowImplCommon blows up otherwise
+    exists(result.getABoundCallbackParameter(_, _))
+  }
+
+  override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
+    preservesValue = true and
+    input = "Argument[0..].ReturnValue[exception]" and
+    output = "ReturnValue[exception]"
+  }
+}

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

@@ -6,7 +6,7 @@ private import javascript
 private import semmle.javascript.dataflow.FlowSummary
 private import FlowSummaryUtil
 
-private DataFlow::SourceNode promiseConstructorRef() {
+DataFlow::SourceNode promiseConstructorRef() {
   result = Promises::promiseConstructorRef()
   or
   result = DataFlow::moduleImport("bluebird")

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

@@ -10,7 +10,7 @@ function e1() {
             throw source('e1.2');
         });
     } catch (err) {
-        sink(err); // $ hasValueFlow=e1.2 hasValueFlow=e1.1
+        sink(err); // $ hasValueFlow=e1.2 MISSING: hasValueFlow=e1.1
     }
 }
 
@@ -24,7 +24,7 @@ function e2() {
             throw source('e2.2');
         });
     } catch (err) {
-        sink(err); // $ MISSING: hasValueFlow=e2.2
+        sink(err); // $ hasValueFlow=e2.2
     }
 }
 
@@ -55,11 +55,11 @@ function e4() {
     try {
         thrower([source("e4.1")]);
     } catch (e) {
-        sink(e); // $ hasValueFlow=e4.1
+        sink(e); // $ MISSING: hasValueFlow=e4.1
     }
     try {
         thrower(["safe"]);
     } catch (e) {
-        sink(e); // $ SPURIOUS: hasValueFlow=e4.1
+        sink(e);
     }
 }