JS: Propagate exceptions from summarized callables by default

Author: asgerf

javascript/ql/lib/semmle/javascript/dataflow/internal/DataFlowNode.qll

@@ -100,6 +100,9 @@ private module Cached {
       //       So it doesn't cause negative recursion but it might look a bit surprising.
       FlowSummaryPrivate::Steps::summaryStoreStep(sn, MkAwaited(), _)
     } or
+    TFlowSummaryDefaultExceptionalReturn(FlowSummaryImpl::Public::SummarizedCallable callable) {
+      not DataFlowPrivate::mentionsExceptionalReturn(callable)
+    } or
     TSynthCaptureNode(VariableCapture::VariableCaptureOutput::SynthesizedCaptureNode node) or
     TGenericSynthesizedNode(AstNode node, string tag, DataFlowPrivate::DataFlowCallable container) {
       any(AdditionalFlowInternal flow).needsSynthesizedNode(node, tag, container)

javascript/ql/lib/semmle/javascript/dataflow/internal/DataFlowPrivate.qll

@@ -112,6 +112,33 @@ class FlowSummaryIntermediateAwaitStoreNode extends DataFlow::Node,
   }
 }
 
+predicate mentionsExceptionalReturn(FlowSummaryImpl::Public::SummarizedCallable callable) {
+  exists(FlowSummaryImpl::Private::SummaryNode node | node.getSummarizedCallable() = callable |
+    FlowSummaryImpl::Private::summaryReturnNode(node, MkExceptionalReturnKind())
+    or
+    FlowSummaryImpl::Private::summaryOutNode(_, node, MkExceptionalReturnKind())
+  )
+}
+
+/**
+ * Exceptional return node in a summarized callable whose summary does not mention `ReturnValue[exception]`.
+ *
+ * By default, every call inside such a callable will forward their exceptional return to the caller's
+ * exceptional return, i.e. exceptions are not caught.
+ */
+class FlowSummaryDefaultExceptionalReturn extends DataFlow::Node,
+  TFlowSummaryDefaultExceptionalReturn
+{
+  private FlowSummaryImpl::Public::SummarizedCallable callable;
+
+  FlowSummaryDefaultExceptionalReturn() { this = TFlowSummaryDefaultExceptionalReturn(callable) }
+
+  FlowSummaryImpl::Public::SummarizedCallable getSummarizedCallable() { result = callable }
+
+  cached
+  override string toString() { result = "[default exceptional return] " + callable }
+}
+
 class CaptureNode extends DataFlow::Node, TSynthCaptureNode {
   /** Gets the underlying node from the variable-capture library. */
   VariableCaptureOutput::SynthesizedCaptureNode getNode() {
@@ -296,6 +323,9 @@ private predicate returnNodeImpl(DataFlow::Node node, ReturnKind kind) {
   )
   or
   FlowSummaryImpl::Private::summaryReturnNode(node.(FlowSummaryNode).getSummaryNode(), kind)
+  or
+  node instanceof FlowSummaryDefaultExceptionalReturn and
+  kind = MkExceptionalReturnKind()
 }
 
 private DataFlow::Node getAnOutNodeImpl(DataFlowCall call, ReturnKind kind) {
@@ -311,6 +341,10 @@ private DataFlow::Node getAnOutNodeImpl(DataFlowCall call, ReturnKind kind) {
   or
   FlowSummaryImpl::Private::summaryOutNode(call.(SummaryCall).getReceiver(),
     result.(FlowSummaryNode).getSummaryNode(), kind)
+  or
+  kind = MkExceptionalReturnKind() and
+  result.(FlowSummaryDefaultExceptionalReturn).getSummarizedCallable() =
+    call.(SummaryCall).getSummarizedCallable()
 }
 
 class ReturnNode extends DataFlow::Node {
@@ -505,6 +539,8 @@ DataFlowCallable nodeGetEnclosingCallable(Node node) {
   or
   result.asLibraryCallable() = node.(FlowSummaryIntermediateAwaitStoreNode).getSummarizedCallable()
   or
+  result.asLibraryCallable() = node.(FlowSummaryDefaultExceptionalReturn).getSummarizedCallable()
+  or
   node = TGenericSynthesizedNode(_, _, result)
 }
 
@@ -865,6 +901,8 @@ class SummaryCall extends DataFlowCall, MkSummaryCall {
 
   /** Gets the receiver node. */
   FlowSummaryImpl::Private::SummaryNode getReceiver() { result = receiver }
+
+  FlowSummaryImpl::Public::SummarizedCallable getSummarizedCallable() { result = enclosingCallable }
 }
 
 /**

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

@@ -13,7 +13,7 @@ function e1() {
             throw source('e1.3'); // Same as e1.2 but without callback parameters
         });
     } catch (err) {
-        sink(err); // $ hasValueFlow=e1.2 hasValueFlow=e1.3 MISSING: hasValueFlow=e1.1
+        sink(err); // $ hasValueFlow=e1.2 hasValueFlow=e1.3 hasValueFlow=e1.1
     }
 }
 
@@ -58,7 +58,7 @@ function e4() {
     try {
         thrower([source("e4.1")]);
     } catch (e) {
-        sink(e); // $ MISSING: hasValueFlow=e4.1
+        sink(e); // $ hasValueFlow=e4.1
     }
     try {
         thrower(["safe"]);