From 0004c28fe8eaceb8ffadaf5a6f60927eb766a3ef Mon Sep 17 00:00:00 2001
From: Erik Krogh Kristensen <erik-krogh@github.com>
Date: Fri, 7 Aug 2020 16:24:39 +0200
Subject: [PATCH 01/14] introduce and use FunctionReturnNode

---
 .../javascript/dataflow/Configuration.qll     |  7 +++-
 .../semmle/javascript/dataflow/DataFlow.qll   | 38 +++++++++++++++++++
 .../src/semmle/javascript/dataflow/Nodes.qll  | 10 +++++
 .../semmle/javascript/dataflow/Sources.qll    |  3 ++
 .../dataflow/internal/DataFlowNode.qll        |  1 +
 .../dataflow/internal/FlowSteps.qll           |  5 ++-
 .../library-tests/DataFlow/flowStep.expected  |  8 ++++
 .../library-tests/DataFlow/sources.expected   | 18 +++++++++
 .../InterProceduralFlow/GermanFlow.expected   |  2 +
 .../TaintTracking.expected                    |  2 +
 .../Promises/AdditionalPromises.expected      |  2 +-
 .../ql/test/library-tests/Promises/flow.js    |  2 +-
 .../library-tests/Promises/tests.expected     | 10 ++---
 .../TypeTracking/ClassStyle.expected          |  2 +
 .../TypeTracking/PredicateStyle.expected      |  2 +
 .../Electron/BrowserObject.expected           |  1 +
 .../frameworks/Express/tests.expected         |  5 +++
 .../frameworks/NodeJSLib/tests.expected       |  2 +
 .../frameworks/connect/tests.expected         |  1 +
 .../frameworks/hapi/tests.expected            |  1 +
 20 files changed, 111 insertions(+), 11 deletions(-)

diff --git a/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll b/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll
index 203b282120ff..bd7942761ed7 100644
--- a/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll
@@ -982,8 +982,8 @@ private predicate appendStep(
 private predicate flowThroughCall(
   DataFlow::Node input, DataFlow::Node output, DataFlow::Configuration cfg, PathSummary summary
 ) {
-  exists(Function f, DataFlow::ValueNode ret |
-    ret.asExpr() = f.getAReturnedExpr() and
+  exists(Function f, DataFlow::FunctionReturnNode ret |
+    ret.getFunction() = f and
     (calls(output, f) or callsBound(output, f, _)) and // Do not consider partial calls
     reachableFromInput(f, output, input, ret, cfg, summary) and
     not isBarrierEdge(cfg, ret, output) and
@@ -1624,6 +1624,9 @@ class MidPathNode extends PathNode, MkMidNode {
     // Skip the exceptional return on functions, as this highlights the entire function.
     nd = any(DataFlow::FunctionNode f).getExceptionalReturn()
     or
+    // Skip the special return node for functions, as this highlights the entire function (and the returned expr is the previous node).
+    nd = any(DataFlow::FunctionNode f).getReturnNode()
+    or
     // Skip the synthetic 'this' node, as a ThisExpr will be the next node anyway
     nd = DataFlow::thisNode(_)
     or
diff --git a/javascript/ql/src/semmle/javascript/dataflow/DataFlow.qll b/javascript/ql/src/semmle/javascript/dataflow/DataFlow.qll
index 62d6849f868f..f1525ad75a26 100644
--- a/javascript/ql/src/semmle/javascript/dataflow/DataFlow.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/DataFlow.qll
@@ -922,6 +922,32 @@ module DataFlow {
     override File getFile() { result = function.getFile() }
   }
 
+  /**
+   * A data flow node representing the values returned by a function.
+   */
+  class FunctionReturnNode extends DataFlow::Node, TFunctionReturnNode {
+    Function function;
+
+    FunctionReturnNode() { this = TFunctionReturnNode(function) }
+
+    override string toString() { result = "return of " + function.describe() }
+
+    override predicate hasLocationInfo(
+      string filepath, int startline, int startcolumn, int endline, int endcolumn
+    ) {
+      function.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
+    }
+
+    override BasicBlock getBasicBlock() { result = function.(ExprOrStmt).getBasicBlock() }
+
+    /**
+     * Gets the function corresponding to this return node.
+     */
+    Function getFunction() { result = function }
+
+    override File getFile() { result = function.getFile() }
+  }
+
   /**
    * A data flow node representing the exceptions thrown by the callee of an invocation.
    */
@@ -1265,6 +1291,13 @@ module DataFlow {
     nd = TExceptionalFunctionReturnNode(function)
   }
 
+  /**
+   * INTERNAL: Use `FunctionReturnNode` instead.
+   */
+  predicate functionReturnNode(DataFlow::Node nd, Function function) {
+    nd = TFunctionReturnNode(function)
+  }
+
   /**
    * Gets the data flow node corresponding the given l-value expression, if
    * such a node exists.
@@ -1460,6 +1493,11 @@ module DataFlow {
         localCall(succExpr, f)
       )
     )
+    or
+    // from returned expr to the FunctionReturnNode.
+    exists(Function f | not f.isAsync() |
+      DataFlow::functionReturnNode(succ, f) and pred = valueNode(f.getAReturnedExpr())
+    )
   }
 
   /**
diff --git a/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll b/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll
index aeb69f251790..81719228eedb 100644
--- a/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll
@@ -491,6 +491,16 @@ class FunctionNode extends DataFlow::ValueNode, DataFlow::SourceNode {
   DataFlow::ExceptionalFunctionReturnNode getExceptionalReturn() {
     DataFlow::exceptionalFunctionReturnNode(result, astNode)
   }
+
+  /**
+   * Gets the data flow node representing the value returned from this function.
+   * 
+   * Note that this differs from `getAReturn()`, in that every function has exactly
+   * one canonical return node, but may have multiple (or zero) returned expressions.
+   * The result of `getAReturn()` is always a predecessor of `getReturnNode()`
+   * in the data-flow graph.
+   */
+  DataFlow::FunctionReturnNode getReturnNode() { DataFlow::functionReturnNode(result, astNode) }
 }
 
 /**
diff --git a/javascript/ql/src/semmle/javascript/dataflow/Sources.qll b/javascript/ql/src/semmle/javascript/dataflow/Sources.qll
index 8b54b5029df4..81bcd16c7bd5 100644
--- a/javascript/ql/src/semmle/javascript/dataflow/Sources.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/Sources.qll
@@ -319,6 +319,9 @@ module SourceNode {
       this = DataFlow::destructuredModuleImportNode(_)
       or
       this = DataFlow::globalAccessPathRootPseudoNode()
+      or
+      // Include return nodes because they model the implicit Promise creation in async functions.
+      DataFlow::functionReturnNode(this, _)
     }
   }
 }
diff --git a/javascript/ql/src/semmle/javascript/dataflow/internal/DataFlowNode.qll b/javascript/ql/src/semmle/javascript/dataflow/internal/DataFlowNode.qll
index 9c14fbb76562..f343f0a8cd5c 100644
--- a/javascript/ql/src/semmle/javascript/dataflow/internal/DataFlowNode.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/internal/DataFlowNode.qll
@@ -27,6 +27,7 @@ newtype TNode =
     exists(decl.getASpecifier().getImportedName())
   } or
   THtmlAttributeNode(HTML::Attribute attr) or
+  TFunctionReturnNode(Function f) or
   TExceptionalFunctionReturnNode(Function f) or
   TExceptionalInvocationReturnNode(InvokeExpr e) or
   TGlobalAccessPathRoot()
diff --git a/javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll b/javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll
index 988a1b7c0594..34f2c72b0424 100644
--- a/javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll
@@ -213,14 +213,15 @@ private module CachedSteps {
 
   /**
    * Holds if there is a flow step from `pred` to `succ` through:
-   * - returning a value from a function call, or
+   * - returning a value from a function call (from the special `FunctionReturnNode`), or
    * - throwing an exception out of a function call, or
    * - the receiver flowing out of a constructor call.
    */
   cached
   predicate returnStep(DataFlow::Node pred, DataFlow::Node succ) {
+    // Note: FlowSteps::CachedSteps::returnStep/2 has copy-paste children
     exists(Function f | calls(succ, f) or callsBound(succ, f, _) |
-      returnExpr(f, pred, _)
+      DataFlow::functionReturnNode(pred, f)
       or
       succ instanceof DataFlow::NewNode and
       DataFlow::thisNode(pred, f)
diff --git a/javascript/ql/test/library-tests/DataFlow/flowStep.expected b/javascript/ql/test/library-tests/DataFlow/flowStep.expected
index cfb92ff63390..2f8e58861ad3 100644
--- a/javascript/ql/test/library-tests/DataFlow/flowStep.expected
+++ b/javascript/ql/test/library-tests/DataFlow/flowStep.expected
@@ -17,10 +17,12 @@
 | sources.js:1:6:1:6 | x | sources.js:1:11:1:11 | x |
 | sources.js:1:6:1:11 | x => x | sources.js:1:5:1:12 | (x => x) |
 | sources.js:1:11:1:11 | x | sources.js:1:1:1:12 | new (x => x) |
+| sources.js:1:11:1:11 | x | sources.js:1:6:1:11 | return of anonymous function |
 | sources.js:3:2:5:1 | functio ... x+19;\\n} | sources.js:3:1:5:2 | (functi ... +19;\\n}) |
 | sources.js:3:11:3:11 | x | sources.js:3:11:3:11 | x |
 | sources.js:3:11:3:11 | x | sources.js:4:10:4:10 | x |
 | sources.js:4:10:4:13 | x+19 | sources.js:3:1:5:6 | (functi ... \\n})(23) |
+| sources.js:4:10:4:13 | x+19 | sources.js:3:2:5:1 | return of anonymous function |
 | sources.js:5:4:5:5 | 23 | sources.js:3:11:3:11 | x |
 | sources.js:9:14:9:18 | array | sources.js:9:14:9:18 | array |
 | sources.js:9:14:9:18 | array | sources.js:10:19:10:23 | array |
@@ -89,7 +91,9 @@
 | tst.js:16:13:16:13 | a | tst.js:16:13:16:13 | a |
 | tst.js:16:13:16:13 | a | tst.js:18:12:18:12 | a |
 | tst.js:18:12:18:12 | a | tst.js:16:1:20:9 | (functi ... ("arg") |
+| tst.js:18:12:18:12 | a | tst.js:16:2:20:1 | return of function f |
 | tst.js:19:10:19:11 | "" | tst.js:16:1:20:9 | (functi ... ("arg") |
+| tst.js:19:10:19:11 | "" | tst.js:16:2:20:1 | return of function f |
 | tst.js:20:4:20:8 | "arg" | tst.js:16:13:16:13 | a |
 | tst.js:22:5:22:25 | readFileSync | tst.js:23:1:23:12 | readFileSync |
 | tst.js:22:7:22:18 | readFileSync | tst.js:22:5:22:25 | readFileSync |
@@ -102,11 +106,13 @@
 | tst.js:28:2:28:1 | x | tst.js:29:3:29:3 | x |
 | tst.js:28:2:29:3 | () =>\\n  x | tst.js:28:1:30:1 | (() =>\\n ... ables\\n) |
 | tst.js:29:3:29:3 | x | tst.js:28:1:30:3 | (() =>\\n ... les\\n)() |
+| tst.js:29:3:29:3 | x | tst.js:28:2:29:3 | return of anonymous function |
 | tst.js:32:1:32:0 | x | tst.js:33:10:33:10 | x |
 | tst.js:32:1:34:1 | functio ... ables\\n} | tst.js:32:10:32:10 | g |
 | tst.js:32:10:32:10 | g | tst.js:35:1:35:1 | g |
 | tst.js:32:10:32:10 | g | tst.js:60:1:60:1 | g |
 | tst.js:32:10:32:10 | g | tst.js:62:4:62:4 | g |
+| tst.js:33:10:33:10 | x | tst.js:32:1:34:1 | return of function g |
 | tst.js:37:5:42:1 | o | tst.js:43:1:43:1 | o |
 | tst.js:37:5:42:1 | o | tst.js:44:1:44:1 | o |
 | tst.js:37:5:42:1 | o | tst.js:61:3:61:3 | o |
@@ -142,6 +148,7 @@
 | tst.js:90:15:90:15 | o | tst.js:90:4:90:11 | { r: z } |
 | tst.js:90:15:90:15 | o | tst.js:90:4:90:15 | { r: z } = o |
 | tst.js:91:10:91:18 | x + y + z | tst.js:87:1:96:2 | (functi ... r: 0\\n}) |
+| tst.js:91:10:91:18 | x + y + z | tst.js:87:2:92:1 | return of anonymous function |
 | tst.js:92:4:96:1 | {\\n  p:  ...  r: 0\\n} | tst.js:87:11:87:24 | { p: x, ...o } |
 | tst.js:98:2:103:1 | functio ...  + z;\\n} | tst.js:98:1:103:2 | (functi ... + z;\\n}) |
 | tst.js:98:11:98:24 | rest | tst.js:99:15:99:18 | rest |
@@ -156,6 +163,7 @@
 | tst.js:101:13:101:16 | rest | tst.js:101:3:101:9 | [ , z ] |
 | tst.js:101:13:101:16 | rest | tst.js:101:3:101:16 | [ , z ] = rest |
 | tst.js:102:10:102:18 | x + y + z | tst.js:98:1:103:17 | (functi ... 3, 0 ]) |
+| tst.js:102:10:102:18 | x + y + z | tst.js:98:2:103:1 | return of anonymous function |
 | tst.js:103:4:103:16 | [ 19, 23, 0 ] | tst.js:98:11:98:24 | [ x, ...rest ] |
 | tst.js:105:1:105:1 | x | tst.js:105:1:105:6 | x ?? y |
 | tst.js:105:6:105:6 | y | tst.js:105:1:105:6 | x ?? y |
diff --git a/javascript/ql/test/library-tests/DataFlow/sources.expected b/javascript/ql/test/library-tests/DataFlow/sources.expected
index 0578eb1777db..021ebc9ae3d0 100644
--- a/javascript/ql/test/library-tests/DataFlow/sources.expected
+++ b/javascript/ql/test/library-tests/DataFlow/sources.expected
@@ -2,8 +2,10 @@
 | arguments.js:1:1:12:4 | (functi ... );\\n})() |
 | arguments.js:1:2:1:1 | this |
 | arguments.js:1:2:12:1 | functio ... , 3);\\n} |
+| arguments.js:1:2:12:1 | return of anonymous function |
 | arguments.js:2:5:2:4 | this |
 | arguments.js:2:5:10:5 | functio ... ;\\n    } |
+| arguments.js:2:5:10:5 | return of function f |
 | arguments.js:2:16:2:16 | x |
 | arguments.js:4:28:4:39 | arguments[0] |
 | arguments.js:5:25:5:36 | arguments[1] |
@@ -14,20 +16,24 @@
 | eval.js:1:1:1:0 | this |
 | eval.js:1:1:1:0 | this |
 | eval.js:1:1:5:1 | functio ... eval`\\n} |
+| eval.js:1:1:5:1 | return of function k |
 | eval.js:3:3:3:6 | eval |
 | eval.js:3:3:3:16 | eval("x = 23") |
 | file://:0:0:0:0 | global access path |
 | sources.js:1:1:1:0 | this |
 | sources.js:1:1:1:12 | new (x => x) |
 | sources.js:1:6:1:6 | x |
+| sources.js:1:6:1:11 | return of anonymous function |
 | sources.js:1:6:1:11 | x => x |
 | sources.js:3:1:5:6 | (functi ... \\n})(23) |
 | sources.js:3:2:3:1 | this |
 | sources.js:3:2:5:1 | functio ... x+19;\\n} |
+| sources.js:3:2:5:1 | return of anonymous function |
 | sources.js:3:11:3:11 | x |
 | sources.js:7:1:7:3 | /x/ |
 | sources.js:9:1:9:0 | this |
 | sources.js:9:1:12:1 | functio ... ey; }\\n} |
+| sources.js:9:1:12:1 | return of function foo |
 | sources.js:9:14:9:18 | array |
 | sources.js:10:12:10:14 | key |
 | sources.js:11:12:11:18 | { key } |
@@ -36,12 +42,14 @@
 | tst2.ts:3:3:3:8 | setX() |
 | tst2.ts:7:1:7:0 | this |
 | tst2.ts:7:1:9:1 | functio ... = 23;\\n} |
+| tst2.ts:7:1:9:1 | return of function setX |
 | tst2.ts:8:3:8:5 | A.x |
 | tst2.ts:11:11:11:13 | A.x |
 | tst2.ts:13:1:13:40 | class S ... ing> {} |
 | tst2.ts:13:26:13:29 | List |
 | tst2.ts:13:39:13:38 | (...arg ... rgs); } |
 | tst2.ts:13:39:13:38 | args |
+| tst2.ts:13:39:13:38 | return of default constructor of class StringList |
 | tst2.ts:13:39:13:38 | super(...args) |
 | tst2.ts:13:39:13:38 | this |
 | tst.js:1:1:1:0 | this |
@@ -50,6 +58,7 @@
 | tst.js:16:1:20:9 | (functi ... ("arg") |
 | tst.js:16:2:16:1 | this |
 | tst.js:16:2:20:1 | functio ... n "";\\n} |
+| tst.js:16:2:20:1 | return of function f |
 | tst.js:16:13:16:13 | a |
 | tst.js:17:7:17:10 | Math |
 | tst.js:17:7:17:17 | Math.random |
@@ -57,13 +66,16 @@
 | tst.js:22:7:22:18 | readFileSync |
 | tst.js:28:1:30:3 | (() =>\\n ... les\\n)() |
 | tst.js:28:2:29:3 | () =>\\n  x |
+| tst.js:28:2:29:3 | return of anonymous function |
 | tst.js:32:1:32:0 | this |
 | tst.js:32:1:34:1 | functio ... ables\\n} |
+| tst.js:32:1:34:1 | return of function g |
 | tst.js:32:12:32:12 | b |
 | tst.js:35:1:35:7 | g(true) |
 | tst.js:37:9:42:1 | {\\n  x:  ... ;\\n  }\\n} |
 | tst.js:39:4:39:3 | this |
 | tst.js:39:4:41:3 | () {\\n    this;\\n  } |
+| tst.js:39:4:41:3 | return of method m |
 | tst.js:43:1:43:3 | o.x |
 | tst.js:44:1:44:3 | o.m |
 | tst.js:44:1:44:5 | o.m() |
@@ -73,6 +85,7 @@
 | tst.js:49:17:49:17 | B |
 | tst.js:50:14:50:13 | this |
 | tst.js:50:14:53:3 | () {\\n   ... et`\\n  } |
+| tst.js:50:14:53:3 | return of constructor of class A |
 | tst.js:51:5:51:13 | super(42) |
 | tst.js:58:1:58:3 | tag |
 | tst.js:61:1:61:5 | ::o.m |
@@ -80,6 +93,7 @@
 | tst.js:62:1:62:4 | o::g |
 | tst.js:64:1:64:0 | this |
 | tst.js:64:1:67:1 | functio ... lysed\\n} |
+| tst.js:64:1:67:1 | return of function h |
 | tst.js:65:3:65:10 | yield 42 |
 | tst.js:66:13:66:25 | function.sent |
 | tst.js:68:12:68:14 | h() |
@@ -87,6 +101,7 @@
 | tst.js:69:1:69:13 | iter.next(23) |
 | tst.js:71:1:71:0 | this |
 | tst.js:71:1:73:1 | async f ... lysed\\n} |
+| tst.js:71:1:73:1 | return of function k |
 | tst.js:72:3:72:11 | await p() |
 | tst.js:72:9:72:9 | p |
 | tst.js:72:9:72:11 | p() |
@@ -97,6 +112,7 @@
 | tst.js:87:1:96:2 | (functi ... r: 0\\n}) |
 | tst.js:87:2:87:1 | this |
 | tst.js:87:2:92:1 | functio ...  + z;\\n} |
+| tst.js:87:2:92:1 | return of anonymous function |
 | tst.js:87:11:87:24 | { p: x, ...o } |
 | tst.js:87:13:87:16 | p: x |
 | tst.js:87:22:87:22 | ...o |
@@ -106,6 +122,7 @@
 | tst.js:98:1:103:17 | (functi ... 3, 0 ]) |
 | tst.js:98:2:98:1 | this |
 | tst.js:98:2:103:1 | functio ...  + z;\\n} |
+| tst.js:98:2:103:1 | return of anonymous function |
 | tst.js:98:11:98:24 | [ x, ...rest ] |
 | tst.js:98:13:98:13 | x |
 | tst.js:98:19:98:22 | ...rest |
@@ -114,6 +131,7 @@
 | tst.js:103:4:103:16 | [ 19, 23, 0 ] |
 | tst.js:107:2:107:1 | this |
 | tst.js:107:2:113:1 | functio ...  v2c;\\n} |
+| tst.js:107:2:113:1 | return of anonymous function |
 | tst.js:108:7:108:9 | v1a |
 | tst.js:108:12:108:20 | v1b = o1b |
 | tst.js:108:18:108:20 | o1b |
diff --git a/javascript/ql/test/library-tests/InterProceduralFlow/GermanFlow.expected b/javascript/ql/test/library-tests/InterProceduralFlow/GermanFlow.expected
index 112ec0ba9fa3..e5e0a9d51256 100644
--- a/javascript/ql/test/library-tests/InterProceduralFlow/GermanFlow.expected
+++ b/javascript/ql/test/library-tests/InterProceduralFlow/GermanFlow.expected
@@ -44,6 +44,8 @@
 | tst2.js:3:17:3:26 | "tainted2" | tst2.js:11:15:11:24 | g(source2) |
 | tst2.js:6:24:6:37 | "also tainted" | tst2.js:10:15:10:24 | g(source1) |
 | tst2.js:6:24:6:37 | "also tainted" | tst2.js:11:15:11:24 | g(source2) |
+| tst3.js:2:17:2:26 | "tainted1" | tst3.js:6:15:6:40 | noRetur ... ource1) |
+| tst3.js:9:19:9:28 | "tainted2" | tst3.js:12:15:12:33 | noReturnTracking2() |
 | tst6.mjs:12:14:12:21 | "source" | tst6.mjs:14:12:14:16 | a.m() |
 | tst6.mjs:16:15:16:23 | "source2" | tst6.mjs:18:13:18:24 | a.m.call(a2) |
 | tst.js:2:17:2:22 | "src1" | tst.js:28:20:28:22 | elt |
diff --git a/javascript/ql/test/library-tests/InterProceduralFlow/TaintTracking.expected b/javascript/ql/test/library-tests/InterProceduralFlow/TaintTracking.expected
index b6122f6e2a4e..3bb7652c6c7d 100644
--- a/javascript/ql/test/library-tests/InterProceduralFlow/TaintTracking.expected
+++ b/javascript/ql/test/library-tests/InterProceduralFlow/TaintTracking.expected
@@ -47,6 +47,8 @@
 | tst2.js:3:17:3:26 | "tainted2" | tst2.js:11:15:11:24 | g(source2) |
 | tst2.js:6:24:6:37 | "also tainted" | tst2.js:10:15:10:24 | g(source1) |
 | tst2.js:6:24:6:37 | "also tainted" | tst2.js:11:15:11:24 | g(source2) |
+| tst3.js:2:17:2:26 | "tainted1" | tst3.js:6:15:6:40 | noRetur ... ource1) |
+| tst3.js:9:19:9:28 | "tainted2" | tst3.js:12:15:12:33 | noReturnTracking2() |
 | tst4.js:2:16:2:24 | "tainted" | tst4.js:15:15:15:31 | id(still_tainted) |
 | tst4.js:2:16:2:24 | "tainted" | tst4.js:16:15:16:28 | p.also_tainted |
 | tst4.js:2:16:2:24 | "tainted" | tst4.js:17:15:17:28 | substr(source) |
diff --git a/javascript/ql/test/library-tests/Promises/AdditionalPromises.expected b/javascript/ql/test/library-tests/Promises/AdditionalPromises.expected
index b3bf0e22e065..d6845266dccb 100644
--- a/javascript/ql/test/library-tests/Promises/AdditionalPromises.expected
+++ b/javascript/ql/test/library-tests/Promises/AdditionalPromises.expected
@@ -40,7 +40,7 @@
 | flow.js:86:23:86:70 | new Pro ... ource)) |
 | flow.js:89:3:89:27 | ("foo", ...  => {}) |
 | flow.js:91:21:91:68 | new Pro ... ource)) |
-| flow.js:100:28:100:75 | new Pro ... ource)) |
+| flow.js:100:34:100:81 | new Pro ... ource)) |
 | flow.js:103:2:103:48 | new Pro ... "BLA")) |
 | flow.js:103:2:103:76 | new Pro ... ource}) |
 | flow.js:105:2:105:48 | new Pro ... "BLA")) |
diff --git a/javascript/ql/test/library-tests/Promises/flow.js b/javascript/ql/test/library-tests/Promises/flow.js
index d92ef541b854..7a0442951e29 100644
--- a/javascript/ql/test/library-tests/Promises/flow.js
+++ b/javascript/ql/test/library-tests/Promises/flow.js
@@ -97,7 +97,7 @@
 			return e;
 		}
 	}
-	var foo = returnsRejected(new Promise((resolve, reject) => reject(source)));
+	var foo = await returnsRejected(new Promise((resolve, reject) => reject(source)));
 	sink(foo); // NOT OK!
 	
 	new Promise((resolve, reject) => reject("BLA")).catch(x => {return source}).then(x => sink(x)); // NOT OK
diff --git a/javascript/ql/test/library-tests/Promises/tests.expected b/javascript/ql/test/library-tests/Promises/tests.expected
index 5cb2d0b2a243..ee2e9a4a26ba 100644
--- a/javascript/ql/test/library-tests/Promises/tests.expected
+++ b/javascript/ql/test/library-tests/Promises/tests.expected
@@ -61,7 +61,7 @@ test_PromiseDefinition_getExecutor
 | flow.js:74:10:74:57 | new Pro ... ource)) | flow.js:74:22:74:56 | (resolv ... source) |
 | flow.js:86:23:86:70 | new Pro ... ource)) | flow.js:86:35:86:69 | (resolv ... source) |
 | flow.js:91:21:91:68 | new Pro ... ource)) | flow.js:91:33:91:67 | (resolv ... source) |
-| flow.js:100:28:100:75 | new Pro ... ource)) | flow.js:100:40:100:74 | (resolv ... source) |
+| flow.js:100:34:100:81 | new Pro ... ource)) | flow.js:100:46:100:80 | (resolv ... source) |
 | flow.js:103:2:103:48 | new Pro ... "BLA")) | flow.js:103:14:103:47 | (resolv ... ("BLA") |
 | flow.js:105:2:105:48 | new Pro ... "BLA")) | flow.js:105:14:105:47 | (resolv ... ("BLA") |
 | flow.js:107:17:107:64 | new Pro ... ource)) | flow.js:107:29:107:63 | (resolv ... source) |
@@ -96,7 +96,7 @@ test_PromiseDefinition
 | flow.js:74:10:74:57 | new Pro ... ource)) |
 | flow.js:86:23:86:70 | new Pro ... ource)) |
 | flow.js:91:21:91:68 | new Pro ... ource)) |
-| flow.js:100:28:100:75 | new Pro ... ource)) |
+| flow.js:100:34:100:81 | new Pro ... ource)) |
 | flow.js:103:2:103:48 | new Pro ... "BLA")) |
 | flow.js:105:2:105:48 | new Pro ... "BLA")) |
 | flow.js:107:17:107:64 | new Pro ... ource)) |
@@ -143,7 +143,7 @@ test_PromiseDefinition_getRejectParameter
 | flow.js:74:10:74:57 | new Pro ... ource)) | flow.js:74:32:74:37 | reject |
 | flow.js:86:23:86:70 | new Pro ... ource)) | flow.js:86:45:86:50 | reject |
 | flow.js:91:21:91:68 | new Pro ... ource)) | flow.js:91:43:91:48 | reject |
-| flow.js:100:28:100:75 | new Pro ... ource)) | flow.js:100:50:100:55 | reject |
+| flow.js:100:34:100:81 | new Pro ... ource)) | flow.js:100:56:100:61 | reject |
 | flow.js:103:2:103:48 | new Pro ... "BLA")) | flow.js:103:24:103:29 | reject |
 | flow.js:105:2:105:48 | new Pro ... "BLA")) | flow.js:105:24:105:29 | reject |
 | flow.js:107:17:107:64 | new Pro ... ource)) | flow.js:107:39:107:44 | reject |
@@ -173,7 +173,7 @@ test_PromiseDefinition_getResolveParameter
 | flow.js:74:10:74:57 | new Pro ... ource)) | flow.js:74:23:74:29 | resolve |
 | flow.js:86:23:86:70 | new Pro ... ource)) | flow.js:86:36:86:42 | resolve |
 | flow.js:91:21:91:68 | new Pro ... ource)) | flow.js:91:34:91:40 | resolve |
-| flow.js:100:28:100:75 | new Pro ... ource)) | flow.js:100:41:100:47 | resolve |
+| flow.js:100:34:100:81 | new Pro ... ource)) | flow.js:100:47:100:53 | resolve |
 | flow.js:103:2:103:48 | new Pro ... "BLA")) | flow.js:103:15:103:21 | resolve |
 | flow.js:105:2:105:48 | new Pro ... "BLA")) | flow.js:105:15:105:21 | resolve |
 | flow.js:107:17:107:64 | new Pro ... ource)) | flow.js:107:30:107:36 | resolve |
@@ -222,7 +222,6 @@ flow
 | flow.js:2:15:2:22 | "source" | flow.js:79:20:79:20 | x |
 | flow.js:2:15:2:22 | "source" | flow.js:84:21:84:21 | e |
 | flow.js:2:15:2:22 | "source" | flow.js:89:45:89:45 | e |
-| flow.js:2:15:2:22 | "source" | flow.js:101:7:101:9 | foo |
 | flow.js:2:15:2:22 | "source" | flow.js:103:93:103:93 | x |
 | flow.js:2:15:2:22 | "source" | flow.js:105:95:105:95 | x |
 | flow.js:2:15:2:22 | "source" | flow.js:109:89:109:89 | x |
@@ -320,6 +319,7 @@ typetrack
 | flow.js:89:3:89:47 | ("foo", ... ink(e)) | flow.js:89:3:89:27 | ("foo", ...  => {}) | copy $PromiseResolveField$ |
 | flow.js:89:3:89:47 | ("foo", ... ink(e)) | flow.js:89:40:89:46 | sink(e) | copy $PromiseResolveField$ |
 | flow.js:89:3:89:47 | ("foo", ... ink(e)) | flow.js:89:40:89:46 | sink(e) | store $PromiseResolveField$ |
+| flow.js:100:12:100:82 | await r ... urce))) | flow.js:100:18:100:82 | returns ... urce))) | load $PromiseResolveField$ |
 | flow.js:103:2:103:76 | new Pro ... ource}) | flow.js:103:2:103:48 | new Pro ... "BLA")) | copy $PromiseResolveField$ |
 | flow.js:103:2:103:95 | new Pro ... ink(x)) | flow.js:103:88:103:94 | sink(x) | copy $PromiseResolveField$ |
 | flow.js:103:2:103:95 | new Pro ... ink(x)) | flow.js:103:88:103:94 | sink(x) | store $PromiseResolveField$ |
diff --git a/javascript/ql/test/library-tests/TypeTracking/ClassStyle.expected b/javascript/ql/test/library-tests/TypeTracking/ClassStyle.expected
index 5eced0f76fd1..1a04300fc3d0 100644
--- a/javascript/ql/test/library-tests/TypeTracking/ClassStyle.expected
+++ b/javascript/ql/test/library-tests/TypeTracking/ClassStyle.expected
@@ -45,8 +45,10 @@ test_DataCallback
 | tst.js:21:1:23:1 | functio ... ata);\\n} |
 | tst.js:30:26:30:27 | cb |
 | tst.js:33:17:33:26 | data => {} |
+| tst.js:37:1:39:1 | return of function getDataCurry |
 | tst.js:38:10:38:19 | data => {} |
 | tst.js:40:32:40:45 | getDataCurry() |
+| tst.js:45:1:47:1 | return of function identity |
 | tst.js:45:19:45:20 | cb |
 | tst.js:48:32:48:60 | identit ... llback) |
 | tst.js:51:1:51:37 | functio ... ata) {} |
diff --git a/javascript/ql/test/library-tests/TypeTracking/PredicateStyle.expected b/javascript/ql/test/library-tests/TypeTracking/PredicateStyle.expected
index 80bf90de0917..53bbd6ec126e 100644
--- a/javascript/ql/test/library-tests/TypeTracking/PredicateStyle.expected
+++ b/javascript/ql/test/library-tests/TypeTracking/PredicateStyle.expected
@@ -52,8 +52,10 @@ dataCallback
 | tst.js:21:1:23:1 | functio ... ata);\\n} |
 | tst.js:30:26:30:27 | cb |
 | tst.js:33:17:33:26 | data => {} |
+| tst.js:37:1:39:1 | return of function getDataCurry |
 | tst.js:38:10:38:19 | data => {} |
 | tst.js:40:32:40:45 | getDataCurry() |
+| tst.js:45:1:47:1 | return of function identity |
 | tst.js:45:19:45:20 | cb |
 | tst.js:48:32:48:60 | identit ... llback) |
 | tst.js:51:1:51:37 | functio ... ata) {} |
diff --git a/javascript/ql/test/library-tests/frameworks/Electron/BrowserObject.expected b/javascript/ql/test/library-tests/frameworks/Electron/BrowserObject.expected
index b2ac72f1e1e0..98b7e86d5d38 100644
--- a/javascript/ql/test/library-tests/frameworks/Electron/BrowserObject.expected
+++ b/javascript/ql/test/library-tests/frameworks/Electron/BrowserObject.expected
@@ -2,6 +2,7 @@
 | electron.js:3:10:3:48 | new Bro ... s: {}}) |
 | electron.js:4:5:4:46 | bv |
 | electron.js:4:10:4:46 | new Bro ... s: {}}) |
+| electron.js:35:1:37:1 | return of function foo |
 | electron.js:35:14:35:14 | x |
 | electron.js:35:14:35:14 | x |
 | electron.js:36:12:36:12 | x |
diff --git a/javascript/ql/test/library-tests/frameworks/Express/tests.expected b/javascript/ql/test/library-tests/frameworks/Express/tests.expected
index 4d78f5079278..d54d70c0aaa0 100644
--- a/javascript/ql/test/library-tests/frameworks/Express/tests.expected
+++ b/javascript/ql/test/library-tests/frameworks/Express/tests.expected
@@ -1867,6 +1867,7 @@ test_RouteSetup_getARouteHandler
 | src/advanced-routehandler-registration.js:163:1:163:33 | app.get ... t("f")) | src/advanced-routehandler-registration.js:163:14:163:32 | routesMap2.get("f") |
 | src/auth.js:4:1:4:53 | app.use ... d' }})) | src/auth.js:4:9:4:52 | basicAu ... rd' }}) |
 | src/csurf-example.js:13:1:13:20 | app.use('/api', api) | src/csurf-example.js:10:11:10:27 | createApiRouter() |
+| src/csurf-example.js:13:1:13:20 | app.use('/api', api) | src/csurf-example.js:29:1:37:1 | return of function createApiRouter |
 | src/csurf-example.js:13:1:13:20 | app.use('/api', api) | src/csurf-example.js:30:16:30:35 | new express.Router() |
 | src/csurf-example.js:16:1:16:51 | app.use ... lse })) | src/csurf-example.js:16:9:16:50 | bodyPar ... alse }) |
 | src/csurf-example.js:17:1:17:23 | app.use ... rser()) | src/csurf-example.js:17:9:17:22 | cookieParser() |
@@ -1880,6 +1881,7 @@ test_RouteSetup_getARouteHandler
 | src/express2.js:3:1:4:77 | router. ... sult }) | src/express2.js:4:32:4:76 | functio ... esult } |
 | src/express2.js:6:1:6:15 | app.use(router) | src/express2.js:2:14:2:23 | e.Router() |
 | src/express3.js:4:1:7:2 | app.get ... l");\\n}) | src/express3.js:4:23:7:1 | functio ... al");\\n} |
+| src/express3.js:12:1:12:21 | app.use ... dler()) | src/express3.js:9:1:11:1 | return of function getHandler |
 | src/express3.js:12:1:12:21 | app.use ... dler()) | src/express3.js:10:12:10:32 | functio ...  res){} |
 | src/express3.js:12:1:12:21 | app.use ... dler()) | src/express3.js:12:9:12:20 | getHandler() |
 | src/express4.js:4:1:9:2 | app.get ... c1);\\n}) | src/express4.js:4:23:9:1 | functio ... ic1);\\n} |
@@ -1888,8 +1890,10 @@ test_RouteSetup_getARouteHandler
 | src/express.js:22:1:32:2 | app.pos ... r');\\n}) | src/express.js:22:30:32:1 | functio ... ar');\\n} |
 | src/express.js:34:1:34:53 | app.get ... andler) | src/exportedHandler.js:1:19:1:55 | functio ... res) {} |
 | src/express.js:34:1:34:53 | app.get ... andler) | src/express.js:34:14:34:52 | require ... handler |
+| src/express.js:39:1:39:21 | app.use ... dler()) | src/express.js:36:1:38:1 | return of function getHandler |
 | src/express.js:39:1:39:21 | app.use ... dler()) | src/express.js:37:12:37:32 | functio ...  res){} |
 | src/express.js:39:1:39:21 | app.use ... dler()) | src/express.js:39:9:39:20 | getHandler() |
+| src/express.js:44:1:44:26 | app.use ... dler()) | src/express.js:41:1:43:1 | return of function getArrowHandler |
 | src/express.js:44:1:44:26 | app.use ... dler()) | src/express.js:42:12:42:28 | (req, res) => f() |
 | src/express.js:44:1:44:26 | app.use ... dler()) | src/express.js:44:9:44:25 | getArrowHandler() |
 | src/express.js:46:1:51:2 | app.pos ... me];\\n}) | src/express.js:46:22:51:1 | functio ... ame];\\n} |
@@ -1908,6 +1912,7 @@ test_RouteSetup_getARouteHandler
 | src/routesetups.js:12:1:12:16 | root.post('', h) | src/routesetups.js:12:15:12:15 | h |
 | src/subrouter.js:4:1:4:26 | app.use ... rotect) | src/subrouter.js:4:19:4:25 | protect |
 | src/subrouter.js:5:1:5:29 | app.use ... uter()) | src/subrouter.js:5:14:5:28 | makeSubRouter() |
+| src/subrouter.js:5:1:5:29 | app.use ... uter()) | src/subrouter.js:7:1:12:1 | return of function makeSubRouter |
 | src/subrouter.js:5:1:5:29 | app.use ... uter()) | src/subrouter.js:8:16:8:31 | express.Router() |
 | src/subrouter.js:9:3:9:35 | router. ... ndler1) | src/subrouter.js:9:27:9:34 | handler1 |
 | src/subrouter.js:10:3:10:41 | router. ... ndler2) | src/subrouter.js:10:33:10:40 | handler2 |
diff --git a/javascript/ql/test/library-tests/frameworks/NodeJSLib/tests.expected b/javascript/ql/test/library-tests/frameworks/NodeJSLib/tests.expected
index 8abd0287f956..6c2a02011fa2 100644
--- a/javascript/ql/test/library-tests/frameworks/NodeJSLib/tests.expected
+++ b/javascript/ql/test/library-tests/frameworks/NodeJSLib/tests.expected
@@ -149,10 +149,12 @@ test_RouteSetup_getARouteHandler
 | createServer.js:4:1:4:47 | require ...  => {}) | createServer.js:4:31:4:46 | (req, res) => {} |
 | src/http.js:4:14:10:2 | http.cr ... foo;\\n}) | src/http.js:4:32:10:1 | functio ... .foo;\\n} |
 | src/http.js:12:1:16:2 | http.cr ... r");\\n}) | src/http.js:12:19:16:1 | functio ... ar");\\n} |
+| src/http.js:57:1:57:31 | http.cr ... dler()) | src/http.js:54:1:56:1 | return of function getHandler |
 | src/http.js:57:1:57:31 | http.cr ... dler()) | src/http.js:55:12:55:30 | function(req,res){} |
 | src/http.js:57:1:57:31 | http.cr ... dler()) | src/http.js:57:19:57:30 | getHandler() |
 | src/http.js:60:1:60:33 | createS ... res){}) | src/http.js:60:14:60:32 | function(req,res){} |
 | src/http.js:62:1:65:2 | http.cr ... 2");\\n}) | src/http.js:62:19:65:1 | functio ... r2");\\n} |
+| src/http.js:70:1:70:36 | http.cr ... dler()) | src/http.js:67:1:69:1 | return of function getArrowHandler |
 | src/http.js:70:1:70:36 | http.cr ... dler()) | src/http.js:68:12:68:27 | (req,res) => f() |
 | src/http.js:70:1:70:36 | http.cr ... dler()) | src/http.js:70:19:70:35 | getArrowHandler() |
 | src/https.js:4:14:10:2 | https.c ... foo;\\n}) | src/https.js:4:33:10:1 | functio ... .foo;\\n} |
diff --git a/javascript/ql/test/library-tests/frameworks/connect/tests.expected b/javascript/ql/test/library-tests/frameworks/connect/tests.expected
index 09fc12218f97..78bfe6107e8e 100644
--- a/javascript/ql/test/library-tests/frameworks/connect/tests.expected
+++ b/javascript/ql/test/library-tests/frameworks/connect/tests.expected
@@ -47,6 +47,7 @@ test_RouteHandler_getAResponseExpr
 test_RouteSetup_getARouteHandler
 | src/test.js:6:1:9:2 | app.use ... o');\\n}) | src/test.js:6:9:9:1 | functio ... oo');\\n} |
 | src/test.js:12:1:12:42 | app.use ... word')) | src/test.js:12:9:12:41 | basicAu ... sword') |
+| src/test.js:17:1:17:21 | app.use ... dler()) | src/test.js:14:1:16:1 | return of function getHandler |
 | src/test.js:17:1:17:21 | app.use ... dler()) | src/test.js:15:12:15:32 | functio ...  res){} |
 | src/test.js:17:1:17:21 | app.use ... dler()) | src/test.js:17:9:17:20 | getHandler() |
 | src/test.js:19:1:19:28 | app.use ... res){}) | src/test.js:19:9:19:27 | function(req,res){} |
diff --git a/javascript/ql/test/library-tests/frameworks/hapi/tests.expected b/javascript/ql/test/library-tests/frameworks/hapi/tests.expected
index a0db859d774d..2579ed7a75aa 100644
--- a/javascript/ql/test/library-tests/frameworks/hapi/tests.expected
+++ b/javascript/ql/test/library-tests/frameworks/hapi/tests.expected
@@ -37,6 +37,7 @@ test_RouteSetup_getARouteHandler
 | src/hapi.js:12:1:15:7 | server2 ...     }}) | src/hapi.js:13:14:15:5 | functio ... n\\n    } |
 | src/hapi.js:17:1:18:2 | server2 ... dler\\n}) | src/hapi.js:17:30:18:1 | functio ... ndler\\n} |
 | src/hapi.js:29:1:29:20 | server2.route(route) | src/hapi.js:20:1:27:1 | functio ... oken;\\n} |
+| src/hapi.js:36:1:36:38 | server2 ... ler()}) | src/hapi.js:33:1:35:1 | return of function getHandler |
 | src/hapi.js:36:1:36:38 | server2 ... ler()}) | src/hapi.js:34:12:34:30 | function (req, h){} |
 | src/hapi.js:36:1:36:38 | server2 ... ler()}) | src/hapi.js:36:25:36:36 | getHandler() |
 test_RouteHandler

From cc94c5ec60350dcad6b173fd631dfd4cd4f796b7 Mon Sep 17 00:00:00 2001
From: Erik Krogh Kristensen <erik-krogh@github.com>
Date: Fri, 7 Aug 2020 16:35:46 +0200
Subject: [PATCH 02/14] remove imprecise return-flow from async functions

---
 .../ql/src/semmle/javascript/dataflow/Configuration.qll       | 4 ++--
 javascript/ql/src/semmle/javascript/dataflow/TrackedNodes.qll | 4 ++--
 .../ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll  | 3 ++-
 .../library-tests/TaintTracking/BasicTaintTracking.expected   | 2 --
 .../library-tests/TaintTracking/DataFlowTracking.expected     | 2 --
 5 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll b/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll
index bd7942761ed7..bf431110a579 100644
--- a/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll
@@ -1019,7 +1019,7 @@ private predicate storeStep(
   isAdditionalStoreStep(pred, succ, prop, cfg) and
   summary = PathSummary::level()
   or
-  exists(Function f, DataFlow::Node mid |
+  exists(Function f, DataFlow::Node mid | not f.isAsync() |
     // `f` stores its parameter `pred` in property `prop` of a value that flows back to the caller,
     // and `succ` is an invocation of `f`
     reachableFromInput(f, succ, pred, mid, cfg, summary) and
@@ -1128,7 +1128,7 @@ private predicate loadStep(
   isAdditionalLoadStep(pred, succ, prop, cfg) and
   summary = PathSummary::level()
   or
-  exists(Function f, DataFlow::Node read |
+  exists(Function f, DataFlow::Node read | not f.isAsync() |
     parameterPropRead(f, succ, pred, prop, read, cfg) and
     reachesReturn(f, read, cfg, summary)
   )
diff --git a/javascript/ql/src/semmle/javascript/dataflow/TrackedNodes.qll b/javascript/ql/src/semmle/javascript/dataflow/TrackedNodes.qll
index beacfb554e6c..607cf56adfdd 100644
--- a/javascript/ql/src/semmle/javascript/dataflow/TrackedNodes.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/TrackedNodes.qll
@@ -193,7 +193,7 @@ private module NodeTracking {
     basicStoreStep(pred, succ, prop) and
     summary = PathSummary::level()
     or
-    exists(Function f, DataFlow::Node mid |
+    exists(Function f, DataFlow::Node mid | not f.isAsync() |
       // `f` stores its parameter `pred` in property `prop` of a value that flows back to the caller,
       // and `succ` is an invocation of `f`
       reachableFromInput(f, succ, pred, mid, summary) and
@@ -216,7 +216,7 @@ private module NodeTracking {
     basicLoadStep(pred, succ, prop) and
     summary = PathSummary::level()
     or
-    exists(Function f, DataFlow::SourceNode parm |
+    exists(Function f, DataFlow::SourceNode parm | not f.isAsync() |
       argumentPassing(succ, pred, f, parm) and
       reachesReturn(f, parm.getAPropertyRead(prop), summary)
     )
diff --git a/javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll b/javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll
index 34f2c72b0424..dd5e76c2d209 100644
--- a/javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll
@@ -67,7 +67,8 @@ predicate localExceptionStep(DataFlow::Node pred, DataFlow::Node succ) {
     or
     DataFlow::exceptionalInvocationReturnNode(pred, expr)
   |
-    succ = expr.getExceptionTarget()
+    succ = expr.getExceptionTarget() and
+    not succ = any(DataFlow::FunctionNode f | f.getFunction().isAsync()).getExceptionalReturn()
   )
 }
 
diff --git a/javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected b/javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected
index 54a90be8a56c..bb717be7f9b5 100644
--- a/javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected
+++ b/javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected
@@ -52,8 +52,6 @@ typeInferenceMismatch
 | exceptions.js:21:17:21:24 | source() | exceptions.js:24:10:24:21 | e.toString() |
 | exceptions.js:21:17:21:24 | source() | exceptions.js:25:10:25:18 | e.message |
 | exceptions.js:21:17:21:24 | source() | exceptions.js:26:10:26:19 | e.fileName |
-| exceptions.js:48:16:48:23 | source() | exceptions.js:50:10:50:10 | e |
-| exceptions.js:59:24:59:31 | source() | exceptions.js:61:12:61:12 | e |
 | exceptions.js:88:6:88:13 | source() | exceptions.js:11:10:11:10 | e |
 | exceptions.js:88:6:88:13 | source() | exceptions.js:32:10:32:10 | e |
 | exceptions.js:88:6:88:13 | source() | exceptions.js:33:10:33:21 | e.toString() |
diff --git a/javascript/ql/test/library-tests/TaintTracking/DataFlowTracking.expected b/javascript/ql/test/library-tests/TaintTracking/DataFlowTracking.expected
index b91d63f6ad0c..f13257ba1395 100644
--- a/javascript/ql/test/library-tests/TaintTracking/DataFlowTracking.expected
+++ b/javascript/ql/test/library-tests/TaintTracking/DataFlowTracking.expected
@@ -31,8 +31,6 @@
 | constructor-calls.js:14:15:14:22 | source() | constructor-calls.js:17:8:17:14 | c.param |
 | constructor-calls.js:14:15:14:22 | source() | constructor-calls.js:25:8:25:14 | d.param |
 | exceptions.js:3:15:3:22 | source() | exceptions.js:5:10:5:10 | e |
-| exceptions.js:48:16:48:23 | source() | exceptions.js:50:10:50:10 | e |
-| exceptions.js:59:24:59:31 | source() | exceptions.js:61:12:61:12 | e |
 | exceptions.js:88:6:88:13 | source() | exceptions.js:11:10:11:10 | e |
 | exceptions.js:93:11:93:18 | source() | exceptions.js:95:10:95:10 | e |
 | exceptions.js:100:13:100:20 | source() | exceptions.js:102:12:102:12 | e |

From 26ef2f34dad31693afe476f3581caa9bb7202b23 Mon Sep 17 00:00:00 2001
From: Erik Krogh Kristensen <erik-krogh@github.com>
Date: Fri, 7 Aug 2020 16:37:01 +0200
Subject: [PATCH 03/14] add precise return-flow for async functions

---
 .../ql/src/semmle/javascript/Promises.qll     | 52 ++++++++++++++
 .../InterProceduralFlow/GermanFlow.expected   |  6 ++
 .../TaintTracking.expected                    |  8 +++
 .../InterProceduralFlow/TrackedNodes.expected | 12 ++++
 .../InterProceduralFlow/async.js              | 71 +++++++++++++++++++
 5 files changed, 149 insertions(+)
 create mode 100644 javascript/ql/test/library-tests/InterProceduralFlow/async.js

diff --git a/javascript/ql/src/semmle/javascript/Promises.qll b/javascript/ql/src/semmle/javascript/Promises.qll
index c45b250dd65e..67aeed3af48d 100644
--- a/javascript/ql/src/semmle/javascript/Promises.qll
+++ b/javascript/ql/src/semmle/javascript/Promises.qll
@@ -455,6 +455,58 @@ private class PromiseTaintStep extends TaintTracking::AdditionalTaintStep {
   }
 }
 
+/**
+ * Defines flow steps for return on async functions.
+ */
+private module AsyncReturnSteps {
+  private predicate valueProp = Promises::valueProp/0;
+
+  private predicate errorProp = Promises::errorProp/0;
+
+  private import semmle.javascript.dataflow.internal.FlowSteps
+
+  /**
+   * A data-flow step for ordinary and exceptional returns from async functions.
+   */
+  private class AsyncReturn extends PreCallGraphStep {
+    // Note: partially copy-paste from FlowSteps::CachedSteps::returnStep/2
+    override predicate storeStep(DataFlow::Node pred, DataFlow::SourceNode succ, string prop) {
+      exists(DataFlow::FunctionNode f | f.getFunction().isAsync() |
+        // ordinary return
+        prop = valueProp() and
+        pred = f.getAReturn() and
+        succ = f.getReturnNode()
+        or
+        // exceptional return
+        prop = errorProp() and
+        exists(Expr expr |
+          expr = any(ThrowStmt throw).getExpr() and
+          pred = expr.flow()
+          or
+          DataFlow::exceptionalInvocationReturnNode(pred, expr)
+        |
+          f.getFunction() = expr.getContainer() and
+          succ = f.getReturnNode() // returns a rejected promise - therefore using the ordinary return node.
+        )
+      )
+    }
+  }
+
+  /**
+   * A data-flow step for ordinary return from an async function in a taint configuration.
+   */
+  private class AsyncTaintReturn extends TaintTracking::AdditionalTaintStep, DataFlow::FunctionNode {
+    Function f;
+
+    AsyncTaintReturn() { this.getFunction() = f and f.isAsync() }
+
+    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+      returnExpr(f, pred, _) and
+      succ.(DataFlow::FunctionReturnNode).getFunction() = f
+    }
+  }
+}
+
 /**
  * Provides classes for working with the `bluebird` library (http://bluebirdjs.com).
  */
diff --git a/javascript/ql/test/library-tests/InterProceduralFlow/GermanFlow.expected b/javascript/ql/test/library-tests/InterProceduralFlow/GermanFlow.expected
index e5e0a9d51256..7219fda288c0 100644
--- a/javascript/ql/test/library-tests/InterProceduralFlow/GermanFlow.expected
+++ b/javascript/ql/test/library-tests/InterProceduralFlow/GermanFlow.expected
@@ -1,6 +1,12 @@
 | a.js:1:15:1:23 | "tainted" | b.js:4:13:4:40 | whoKnow ... Tainted |
 | a.js:1:15:1:23 | "tainted" | b.js:6:13:6:13 | x |
 | a.js:2:15:2:28 | "also tainted" | b.js:5:13:5:29 | notTaintedTrustMe |
+| async.js:2:16:2:23 | "source" | async.js:8:15:8:27 | await async() |
+| async.js:2:16:2:23 | "source" | async.js:13:15:13:20 | sync() |
+| async.js:2:16:2:23 | "source" | async.js:27:17:27:17 | e |
+| async.js:2:16:2:23 | "source" | async.js:36:17:36:17 | e |
+| async.js:2:16:2:23 | "source" | async.js:41:17:41:17 | e |
+| async.js:2:16:2:23 | "source" | async.js:54:17:54:36 | unpack(pack(source)) |
 | callback.js:16:14:16:21 | "source" | callback.js:13:14:13:14 | x |
 | callback.js:17:15:17:23 | "source2" | callback.js:13:14:13:14 | x |
 | callback.js:27:15:27:23 | "source3" | callback.js:13:14:13:14 | x |
diff --git a/javascript/ql/test/library-tests/InterProceduralFlow/TaintTracking.expected b/javascript/ql/test/library-tests/InterProceduralFlow/TaintTracking.expected
index 3bb7652c6c7d..de3671470e94 100644
--- a/javascript/ql/test/library-tests/InterProceduralFlow/TaintTracking.expected
+++ b/javascript/ql/test/library-tests/InterProceduralFlow/TaintTracking.expected
@@ -1,6 +1,14 @@
 | a.js:1:15:1:23 | "tainted" | b.js:4:13:4:40 | whoKnow ... Tainted |
 | a.js:1:15:1:23 | "tainted" | b.js:6:13:6:13 | x |
 | a.js:2:15:2:28 | "also tainted" | b.js:5:13:5:29 | notTaintedTrustMe |
+| async.js:2:16:2:23 | "source" | async.js:7:14:7:20 | async() |
+| async.js:2:16:2:23 | "source" | async.js:8:15:8:27 | await async() |
+| async.js:2:16:2:23 | "source" | async.js:13:15:13:20 | sync() |
+| async.js:2:16:2:23 | "source" | async.js:14:15:14:26 | await sync() |
+| async.js:2:16:2:23 | "source" | async.js:27:17:27:17 | e |
+| async.js:2:16:2:23 | "source" | async.js:36:17:36:17 | e |
+| async.js:2:16:2:23 | "source" | async.js:41:17:41:17 | e |
+| async.js:2:16:2:23 | "source" | async.js:54:17:54:36 | unpack(pack(source)) |
 | callback.js:16:14:16:21 | "source" | callback.js:13:14:13:14 | x |
 | callback.js:17:15:17:23 | "source2" | callback.js:13:14:13:14 | x |
 | callback.js:27:15:27:23 | "source3" | callback.js:13:14:13:14 | x |
diff --git a/javascript/ql/test/library-tests/InterProceduralFlow/TrackedNodes.expected b/javascript/ql/test/library-tests/InterProceduralFlow/TrackedNodes.expected
index be2f59884327..5f940e2a3a32 100644
--- a/javascript/ql/test/library-tests/InterProceduralFlow/TrackedNodes.expected
+++ b/javascript/ql/test/library-tests/InterProceduralFlow/TrackedNodes.expected
@@ -1,3 +1,11 @@
+| missing | async.js:1:2:1:2 | source | async.js:8:15:8:27 | await async() |
+| missing | async.js:1:2:1:2 | source | async.js:26:12:26:12 | e |
+| missing | async.js:1:2:1:2 | source | async.js:26:12:26:12 | e |
+| missing | async.js:1:2:1:2 | source | async.js:27:17:27:17 | e |
+| missing | async.js:2:16:2:23 | "source" | async.js:8:15:8:27 | await async() |
+| missing | async.js:2:16:2:23 | "source" | async.js:26:12:26:12 | e |
+| missing | async.js:2:16:2:23 | "source" | async.js:26:12:26:12 | e |
+| missing | async.js:2:16:2:23 | "source" | async.js:27:17:27:17 | e |
 | missing | callback.js:17:15:17:23 | "source2" | callback.js:8:16:8:20 | xs[i] |
 | missing | callback.js:17:15:17:23 | "source2" | callback.js:12:16:12:16 | x |
 | missing | callback.js:17:15:17:23 | "source2" | callback.js:12:16:12:16 | x |
@@ -53,3 +61,7 @@
 | missing | tst.js:2:17:2:22 | "src1" | tst.js:27:22:27:24 | elt |
 | missing | tst.js:2:17:2:22 | "src1" | tst.js:27:22:27:24 | elt |
 | missing | tst.js:2:17:2:22 | "src1" | tst.js:28:20:28:22 | elt |
+| spurious | async.js:1:2:1:2 | source | async.js:7:14:7:20 | async() |
+| spurious | async.js:1:2:1:2 | source | async.js:8:21:8:27 | async() |
+| spurious | async.js:2:16:2:23 | "source" | async.js:7:14:7:20 | async() |
+| spurious | async.js:2:16:2:23 | "source" | async.js:8:21:8:27 | async() |
diff --git a/javascript/ql/test/library-tests/InterProceduralFlow/async.js b/javascript/ql/test/library-tests/InterProceduralFlow/async.js
new file mode 100644
index 000000000000..5859fa69f2e9
--- /dev/null
+++ b/javascript/ql/test/library-tests/InterProceduralFlow/async.js
@@ -0,0 +1,71 @@
+(async function () {
+  let source = "source";
+
+  async function async() {
+    return source;
+  }
+  let sink = async(); // OK - wrapped in a promise. (NOT OK for taint-tracking configs)
+  let sink2 = await async(); // NOT OK
+
+  function sync() {
+    return source;
+  }
+  let sink3 = sync(); // NOT OK
+  let sink4 = await sync(); // OK
+
+  async function throwsAsync() {
+    throw source;
+  }
+  try {
+    throwsAsync();
+  } catch (e) {
+    let sink5 = e; // OK - throwsAsync just returns a promise.
+  }
+  try {
+    await throwsAsync();
+  } catch (e) {
+    let sink6 = e; // NOT OK
+  }
+
+  function throws() {
+    throw source;
+  }
+  try {
+    throws();
+  } catch (e) {
+    let sink5 = e; // NOT OK
+  }
+  try {
+    await throws();
+  } catch (e) {
+    let sink6 = e; // NOT OK
+  }
+
+  function syncTest() {
+    function pack(x) {
+      return {
+        x: x
+      }
+    };
+    function unpack(x) {
+      return x.x;
+    }
+
+    var sink7 = unpack(pack(source)); // NOT OK
+  }
+
+  function asyncTest() {
+    async function pack(x) {
+      return {
+        x: x
+      }
+    };
+    function unpack(x) {
+      return x.x;
+    }
+
+    var sink8 = unpack(pack(source)); // OK 
+    let sink9 = unpack(await (pack(source))); // NOT OK - but not found
+  }
+})();
+

From 0edb46c20d3404f75f185bc1f7a52118e69959f9 Mon Sep 17 00:00:00 2001
From: Erik Krogh Kristensen <erik-krogh@github.com>
Date: Fri, 7 Aug 2020 16:38:01 +0200
Subject: [PATCH 04/14] improve precision for load/store steps with async
 functions

---
 .../javascript/dataflow/Configuration.qll     | 30 ++++++++++++++++++
 .../InterProceduralFlow/GermanFlow.expected   |  3 ++
 .../TaintTracking.expected                    |  3 ++
 .../InterProceduralFlow/TrackedNodes.expected |  9 ++++++
 .../InterProceduralFlow/async.js              | 31 +++++++++++++++++++
 .../ql/test/library-tests/Promises/flow.js    | 27 +++++++++++++++-
 .../library-tests/Promises/tests.expected     |  6 ++++
 7 files changed, 108 insertions(+), 1 deletion(-)

diff --git a/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll b/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll
index bf431110a579..61378d86609e 100644
--- a/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll
@@ -1034,6 +1034,25 @@ private predicate storeStep(
       receiverPropWrite(f, prop, mid)
     )
   )
+  or
+  // store in an immediately awaited function call
+  exists(Function f, DataFlow::Node mid | f.isAsync() |
+    // `f` stores its parameter `pred` in property `prop` of a value that flows back to the caller,
+    // and `succ` is an invocation of `f`
+    exists(AwaitExpr await, DataFlow::Node operand |
+      operand = await.getOperand().getUnderlyingValue().flow() and
+      succ.asExpr() = await
+    |
+      reachableFromInput(f, operand, pred, mid, cfg, summary) and
+      (
+        returnedPropWrite(f, _, prop, mid)
+        or
+        exists(DataFlow::SourceNode base | base.flowsToExpr(f.getAReturnedExpr()) |
+          isAdditionalStoreStep(mid, base, prop, cfg)
+        )
+      )
+    )
+  )
 }
 
 /**
@@ -1132,6 +1151,17 @@ private predicate loadStep(
     parameterPropRead(f, succ, pred, prop, read, cfg) and
     reachesReturn(f, read, cfg, summary)
   )
+  or
+  // load from an immediately awaited function call
+  exists(Function f, DataFlow::Node read | f.isAsync() |
+    exists(AwaitExpr await, DataFlow::Node operand |
+      operand = await.getOperand().getUnderlyingValue().flow() and
+      succ.asExpr() = await
+    |
+      parameterPropRead(f, operand, pred, prop, read, cfg) and
+      reachesReturn(f, read, cfg, summary)
+    )
+  )
 }
 
 /**
diff --git a/javascript/ql/test/library-tests/InterProceduralFlow/GermanFlow.expected b/javascript/ql/test/library-tests/InterProceduralFlow/GermanFlow.expected
index 7219fda288c0..2c5dbcf44e15 100644
--- a/javascript/ql/test/library-tests/InterProceduralFlow/GermanFlow.expected
+++ b/javascript/ql/test/library-tests/InterProceduralFlow/GermanFlow.expected
@@ -7,6 +7,9 @@
 | async.js:2:16:2:23 | "source" | async.js:36:17:36:17 | e |
 | async.js:2:16:2:23 | "source" | async.js:41:17:41:17 | e |
 | async.js:2:16:2:23 | "source" | async.js:54:17:54:36 | unpack(pack(source)) |
+| async.js:79:16:79:23 | "source" | async.js:80:14:80:36 | (await  ... ce))).p |
+| async.js:79:16:79:23 | "source" | async.js:92:15:92:30 | await (getP(o3)) |
+| async.js:96:18:96:25 | "source" | async.js:101:15:101:27 | await readP() |
 | callback.js:16:14:16:21 | "source" | callback.js:13:14:13:14 | x |
 | callback.js:17:15:17:23 | "source2" | callback.js:13:14:13:14 | x |
 | callback.js:27:15:27:23 | "source3" | callback.js:13:14:13:14 | x |
diff --git a/javascript/ql/test/library-tests/InterProceduralFlow/TaintTracking.expected b/javascript/ql/test/library-tests/InterProceduralFlow/TaintTracking.expected
index de3671470e94..86144ae0947d 100644
--- a/javascript/ql/test/library-tests/InterProceduralFlow/TaintTracking.expected
+++ b/javascript/ql/test/library-tests/InterProceduralFlow/TaintTracking.expected
@@ -9,6 +9,9 @@
 | async.js:2:16:2:23 | "source" | async.js:36:17:36:17 | e |
 | async.js:2:16:2:23 | "source" | async.js:41:17:41:17 | e |
 | async.js:2:16:2:23 | "source" | async.js:54:17:54:36 | unpack(pack(source)) |
+| async.js:79:16:79:23 | "source" | async.js:80:14:80:36 | (await  ... ce))).p |
+| async.js:79:16:79:23 | "source" | async.js:92:15:92:30 | await (getP(o3)) |
+| async.js:96:18:96:25 | "source" | async.js:101:15:101:27 | await readP() |
 | callback.js:16:14:16:21 | "source" | callback.js:13:14:13:14 | x |
 | callback.js:17:15:17:23 | "source2" | callback.js:13:14:13:14 | x |
 | callback.js:27:15:27:23 | "source3" | callback.js:13:14:13:14 | x |
diff --git a/javascript/ql/test/library-tests/InterProceduralFlow/TrackedNodes.expected b/javascript/ql/test/library-tests/InterProceduralFlow/TrackedNodes.expected
index 5f940e2a3a32..640000ef0064 100644
--- a/javascript/ql/test/library-tests/InterProceduralFlow/TrackedNodes.expected
+++ b/javascript/ql/test/library-tests/InterProceduralFlow/TrackedNodes.expected
@@ -6,6 +6,15 @@
 | missing | async.js:2:16:2:23 | "source" | async.js:26:12:26:12 | e |
 | missing | async.js:2:16:2:23 | "source" | async.js:26:12:26:12 | e |
 | missing | async.js:2:16:2:23 | "source" | async.js:27:17:27:17 | e |
+| missing | async.js:73:22:73:22 | x | async.js:80:14:80:36 | (await  ... ce))).p |
+| missing | async.js:74:12:76:5 | {\\n      p: x\\n    } | async.js:80:14:80:34 | (await  ... urce))) |
+| missing | async.js:74:12:76:5 | {\\n      p: x\\n    } | async.js:80:15:80:33 | await (foo(source)) |
+| missing | async.js:79:16:79:23 | "source" | async.js:80:14:80:36 | (await  ... ce))).p |
+| missing | async.js:79:16:79:23 | "source" | async.js:92:15:92:30 | await (getP(o3)) |
+| missing | async.js:84:12:84:17 | base.p | async.js:92:15:92:30 | await (getP(o3)) |
+| missing | async.js:88:12:88:17 | base.q | async.js:93:15:93:30 | await (getQ(o3)) |
+| missing | async.js:96:18:96:25 | "source" | async.js:101:15:101:27 | await readP() |
+| missing | async.js:98:12:98:16 | obj.x | async.js:101:15:101:27 | await readP() |
 | missing | callback.js:17:15:17:23 | "source2" | callback.js:8:16:8:20 | xs[i] |
 | missing | callback.js:17:15:17:23 | "source2" | callback.js:12:16:12:16 | x |
 | missing | callback.js:17:15:17:23 | "source2" | callback.js:12:16:12:16 | x |
diff --git a/javascript/ql/test/library-tests/InterProceduralFlow/async.js b/javascript/ql/test/library-tests/InterProceduralFlow/async.js
index 5859fa69f2e9..f91cda9cea85 100644
--- a/javascript/ql/test/library-tests/InterProceduralFlow/async.js
+++ b/javascript/ql/test/library-tests/InterProceduralFlow/async.js
@@ -69,3 +69,34 @@
   }
 })();
 
+async function props() {
+  async function foo(x) {
+    return {
+      p: x
+    };
+  }
+  
+  let source = "source";
+  let sink = (await (foo(source))).p; // NOT OK - this requires the immidiatly awaited storeStep.
+  let sink2 = foo("not a source").p;
+  
+  async function getP(base) {
+    return base.p;
+  }
+  
+  async function getQ(base) {
+    return base.q;
+  }
+  
+  let o3 = { p: source };
+  let sink6 = await (getP(o3)); // NOT OK - this requires the immidiatly awaited loadStep
+  let sink7 = await (getQ(o3));
+
+  async function readP() {
+    let source = "source";
+    var obj = {x: source};
+    return obj.x;
+  }
+
+  let sink8 = await readP(); // NOT OK
+}
diff --git a/javascript/ql/test/library-tests/Promises/flow.js b/javascript/ql/test/library-tests/Promises/flow.js
index 7a0442951e29..ff8040f6e9d4 100644
--- a/javascript/ql/test/library-tests/Promises/flow.js
+++ b/javascript/ql/test/library-tests/Promises/flow.js
@@ -129,4 +129,29 @@
 	new Promise((resolve, reject) => resolve(resolved)).then(x => sink(x)); // NOT OK
 	
 	Promise.resolve(resolved).then(x => sink(x)); // NOT OK
-})();
\ No newline at end of file
+})();
+
+
+(async function () {
+	var source = "source";
+  
+	async function async() {
+	  return source;
+	}
+	sink(async()); // OK - wrapped in a promise. (NOT OK for taint-tracking configs)
+	sink(await async()); // NOT OK
+  
+	async function throwsAsync() {
+	  throw source;
+	}
+	try {
+	  throwsAsync();
+	} catch (e) {
+	  sink(e); // OK - throwsAsync just returns a promise.
+	}
+	try {
+	  await throwsAsync();
+	} catch (e) {
+	  sink(e); // NOT OK
+	}
+  })();
\ No newline at end of file
diff --git a/javascript/ql/test/library-tests/Promises/tests.expected b/javascript/ql/test/library-tests/Promises/tests.expected
index ee2e9a4a26ba..69e3fd093bc9 100644
--- a/javascript/ql/test/library-tests/Promises/tests.expected
+++ b/javascript/ql/test/library-tests/Promises/tests.expected
@@ -222,6 +222,7 @@ flow
 | flow.js:2:15:2:22 | "source" | flow.js:79:20:79:20 | x |
 | flow.js:2:15:2:22 | "source" | flow.js:84:21:84:21 | e |
 | flow.js:2:15:2:22 | "source" | flow.js:89:45:89:45 | e |
+| flow.js:2:15:2:22 | "source" | flow.js:101:7:101:9 | foo |
 | flow.js:2:15:2:22 | "source" | flow.js:103:93:103:93 | x |
 | flow.js:2:15:2:22 | "source" | flow.js:105:95:105:95 | x |
 | flow.js:2:15:2:22 | "source" | flow.js:109:89:109:89 | x |
@@ -231,8 +232,11 @@ flow
 | flow.js:2:15:2:22 | "source" | flow.js:125:59:125:59 | x |
 | flow.js:2:15:2:22 | "source" | flow.js:129:69:129:69 | x |
 | flow.js:2:15:2:22 | "source" | flow.js:131:43:131:43 | x |
+| flow.js:136:15:136:22 | "source" | flow.js:142:7:142:19 | await async() |
+| flow.js:136:15:136:22 | "source" | flow.js:155:9:155:9 | e |
 exclusiveTaintFlow
 | flow2.js:2:15:2:22 | "source" | flow2.js:20:7:20:14 | tainted3 |
+| flow.js:136:15:136:22 | "source" | flow.js:141:7:141:13 | async() |
 | interflow.js:3:18:3:25 | "source" | interflow.js:18:10:18:14 | error |
 typetrack
 | flow2.js:4:2:4:31 | Promise ... lean"]) | flow2.js:4:14:4:30 | [source, "clean"] | copy $PromiseResolveField$ |
@@ -370,6 +374,8 @@ typetrack
 | flow.js:131:2:131:45 | Promise ... ink(x)) | flow.js:131:38:131:44 | sink(x) | copy $PromiseResolveField$ |
 | flow.js:131:2:131:45 | Promise ... ink(x)) | flow.js:131:38:131:44 | sink(x) | store $PromiseResolveField$ |
 | flow.js:131:33:131:33 | x | flow.js:131:2:131:26 | Promise ... solved) | load $PromiseResolveField$ |
+| flow.js:142:7:142:19 | await async() | flow.js:142:13:142:19 | async() | load $PromiseResolveField$ |
+| flow.js:153:4:153:22 | await throwsAsync() | flow.js:153:10:153:22 | throwsAsync() | load $PromiseResolveField$ |
 | interflow.js:6:3:9:23 | loadScr ... eError) | interflow.js:6:3:8:26 | loadScr ... () { }) | copy $PromiseResolveField$ |
 | promises.js:33:19:35:6 | new Pro ... \\n    }) | promises.js:34:17:34:22 | source | copy $PromiseResolveField$ |
 | promises.js:33:19:35:6 | new Pro ... \\n    }) | promises.js:34:17:34:22 | source | store $PromiseResolveField$ |

From b9a98f51ea25aa5fe55689ee7df70016333cadf1 Mon Sep 17 00:00:00 2001
From: Erik Krogh Kristensen <erik-krogh@github.com>
Date: Fri, 7 Aug 2020 16:39:06 +0200
Subject: [PATCH 05/14] update existing tests to work with FunctionReturnNode

---
 .../InterProceduralFlow/DataFlowConfig.qll          | 13 +++++++------
 .../InterProceduralFlow/GermanFlow.expected         |  2 --
 .../InterProceduralFlow/TaintTracking.expected      |  2 --
 .../InterProceduralFlow/TaintTracking.ql            | 13 +++++++------
 4 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/javascript/ql/test/library-tests/InterProceduralFlow/DataFlowConfig.qll b/javascript/ql/test/library-tests/InterProceduralFlow/DataFlowConfig.qll
index 7e7794961755..eccf834a0e15 100644
--- a/javascript/ql/test/library-tests/InterProceduralFlow/DataFlowConfig.qll
+++ b/javascript/ql/test/library-tests/InterProceduralFlow/DataFlowConfig.qll
@@ -17,14 +17,15 @@ class TestDataFlowConfiguration extends DataFlow::Configuration {
     )
   }
 
-  override predicate isBarrierEdge(DataFlow::Node src, DataFlow::Node snk) {
-    src = src and
-    snk.asExpr().(PropAccess).getPropertyName() = "notTracked"
-    or
+  override predicate isBarrier(DataFlow::Node node) {
     exists(Function f |
       f.getName().matches("%noReturnTracking%") and
-      src = f.getAReturnedExpr().flow() and
-      snk.(DataFlow::InvokeNode).getACallee() = f
+      node = f.getAReturnedExpr().flow()
     )
   }
+
+  override predicate isBarrierEdge(DataFlow::Node src, DataFlow::Node snk) {
+    src = src and
+    snk.asExpr().(PropAccess).getPropertyName() = "notTracked"
+  }
 }
diff --git a/javascript/ql/test/library-tests/InterProceduralFlow/GermanFlow.expected b/javascript/ql/test/library-tests/InterProceduralFlow/GermanFlow.expected
index 2c5dbcf44e15..f5f0614794fc 100644
--- a/javascript/ql/test/library-tests/InterProceduralFlow/GermanFlow.expected
+++ b/javascript/ql/test/library-tests/InterProceduralFlow/GermanFlow.expected
@@ -53,8 +53,6 @@
 | tst2.js:3:17:3:26 | "tainted2" | tst2.js:11:15:11:24 | g(source2) |
 | tst2.js:6:24:6:37 | "also tainted" | tst2.js:10:15:10:24 | g(source1) |
 | tst2.js:6:24:6:37 | "also tainted" | tst2.js:11:15:11:24 | g(source2) |
-| tst3.js:2:17:2:26 | "tainted1" | tst3.js:6:15:6:40 | noRetur ... ource1) |
-| tst3.js:9:19:9:28 | "tainted2" | tst3.js:12:15:12:33 | noReturnTracking2() |
 | tst6.mjs:12:14:12:21 | "source" | tst6.mjs:14:12:14:16 | a.m() |
 | tst6.mjs:16:15:16:23 | "source2" | tst6.mjs:18:13:18:24 | a.m.call(a2) |
 | tst.js:2:17:2:22 | "src1" | tst.js:28:20:28:22 | elt |
diff --git a/javascript/ql/test/library-tests/InterProceduralFlow/TaintTracking.expected b/javascript/ql/test/library-tests/InterProceduralFlow/TaintTracking.expected
index 86144ae0947d..eeac69e3af17 100644
--- a/javascript/ql/test/library-tests/InterProceduralFlow/TaintTracking.expected
+++ b/javascript/ql/test/library-tests/InterProceduralFlow/TaintTracking.expected
@@ -58,8 +58,6 @@
 | tst2.js:3:17:3:26 | "tainted2" | tst2.js:11:15:11:24 | g(source2) |
 | tst2.js:6:24:6:37 | "also tainted" | tst2.js:10:15:10:24 | g(source1) |
 | tst2.js:6:24:6:37 | "also tainted" | tst2.js:11:15:11:24 | g(source2) |
-| tst3.js:2:17:2:26 | "tainted1" | tst3.js:6:15:6:40 | noRetur ... ource1) |
-| tst3.js:9:19:9:28 | "tainted2" | tst3.js:12:15:12:33 | noReturnTracking2() |
 | tst4.js:2:16:2:24 | "tainted" | tst4.js:15:15:15:31 | id(still_tainted) |
 | tst4.js:2:16:2:24 | "tainted" | tst4.js:16:15:16:28 | p.also_tainted |
 | tst4.js:2:16:2:24 | "tainted" | tst4.js:17:15:17:28 | substr(source) |
diff --git a/javascript/ql/test/library-tests/InterProceduralFlow/TaintTracking.ql b/javascript/ql/test/library-tests/InterProceduralFlow/TaintTracking.ql
index f9b1a552086b..ae4ab0cd6a5a 100644
--- a/javascript/ql/test/library-tests/InterProceduralFlow/TaintTracking.ql
+++ b/javascript/ql/test/library-tests/InterProceduralFlow/TaintTracking.ql
@@ -17,16 +17,17 @@ class TestTaintTrackingConfiguration extends TaintTracking::Configuration {
     )
   }
 
-  override predicate isSanitizerEdge(DataFlow::Node src, DataFlow::Node snk) {
-    src = src and
-    snk.asExpr().(PropAccess).getPropertyName() = "notTracked"
-    or
+  override predicate isSanitizer(DataFlow::Node node) {
     exists(Function f |
       f.getName().matches("%noReturnTracking%") and
-      src = f.getAReturnedExpr().flow() and
-      snk.(DataFlow::InvokeNode).getACallee() = f
+      node = f.getAReturnedExpr().flow()
     )
   }
+
+  override predicate isSanitizerEdge(DataFlow::Node src, DataFlow::Node snk) {
+    src = src and
+    snk.asExpr().(PropAccess).getPropertyName() = "notTracked"
+  }
 }
 
 from TestTaintTrackingConfiguration tttc, DataFlow::Node src, DataFlow::Node snk

From 8f06e9651ffd642c6e856142e29f16525c2a556f Mon Sep 17 00:00:00 2001
From: Erik Krogh Kristensen <erik-krogh@github.com>
Date: Fri, 7 Aug 2020 16:40:18 +0200
Subject: [PATCH 06/14] update expected output

---
 .../library-tests/InterProceduralFlow/DataFlow.expected  | 9 +++++++++
 .../ql/test/library-tests/TaintTracking/exceptions.js    | 4 ++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/javascript/ql/test/library-tests/InterProceduralFlow/DataFlow.expected b/javascript/ql/test/library-tests/InterProceduralFlow/DataFlow.expected
index a7f2e7ce5af0..c246c74b005d 100644
--- a/javascript/ql/test/library-tests/InterProceduralFlow/DataFlow.expected
+++ b/javascript/ql/test/library-tests/InterProceduralFlow/DataFlow.expected
@@ -1,6 +1,15 @@
 | a.js:1:15:1:23 | "tainted" | b.js:4:13:4:40 | whoKnow ... Tainted |
 | a.js:1:15:1:23 | "tainted" | b.js:6:13:6:13 | x |
 | a.js:2:15:2:28 | "also tainted" | b.js:5:13:5:29 | notTaintedTrustMe |
+| async.js:2:16:2:23 | "source" | async.js:8:15:8:27 | await async() |
+| async.js:2:16:2:23 | "source" | async.js:13:15:13:20 | sync() |
+| async.js:2:16:2:23 | "source" | async.js:27:17:27:17 | e |
+| async.js:2:16:2:23 | "source" | async.js:36:17:36:17 | e |
+| async.js:2:16:2:23 | "source" | async.js:41:17:41:17 | e |
+| async.js:2:16:2:23 | "source" | async.js:54:17:54:36 | unpack(pack(source)) |
+| async.js:81:16:81:23 | "source" | async.js:82:14:82:36 | (await  ... ce))).p |
+| async.js:81:16:81:23 | "source" | async.js:94:15:94:30 | await (getP(o3)) |
+| async.js:98:18:98:25 | "source" | async.js:103:15:103:27 | await readP() |
 | callback.js:16:14:16:21 | "source" | callback.js:13:14:13:14 | x |
 | callback.js:17:15:17:23 | "source2" | callback.js:13:14:13:14 | x |
 | callback.js:27:15:27:23 | "source3" | callback.js:13:14:13:14 | x |
diff --git a/javascript/ql/test/library-tests/TaintTracking/exceptions.js b/javascript/ql/test/library-tests/TaintTracking/exceptions.js
index 98e3e7ce0091..72d822be9ada 100644
--- a/javascript/ql/test/library-tests/TaintTracking/exceptions.js
+++ b/javascript/ql/test/library-tests/TaintTracking/exceptions.js
@@ -47,7 +47,7 @@ function test(unsafe, safe) {
   try {
     throwAsync(source());
   } catch (e) {
-    sink(e); // OK - but flagged anyway
+    sink(e); // OK
   }
 
   throwAsync(source()).catch(e => {
@@ -58,7 +58,7 @@ function test(unsafe, safe) {
     try {
       await throwAsync(source());
     } catch (e) {
-      sink(e); // NOT OK
+      sink(e); // NOT OK - but not flagged
     }
   }
 }

From 94cf3a8ddba48d74a0a0e220e69c35dc8f78edf9 Mon Sep 17 00:00:00 2001
From: Erik Krogh Kristensen <erik-krogh@github.com>
Date: Fri, 7 Aug 2020 17:48:55 +0200
Subject: [PATCH 07/14] correct copy-paste note after refactorings

---
 javascript/ql/src/semmle/javascript/Promises.qll                | 2 +-
 .../ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll    | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/javascript/ql/src/semmle/javascript/Promises.qll b/javascript/ql/src/semmle/javascript/Promises.qll
index 67aeed3af48d..9a113e64e6bd 100644
--- a/javascript/ql/src/semmle/javascript/Promises.qll
+++ b/javascript/ql/src/semmle/javascript/Promises.qll
@@ -469,7 +469,6 @@ private module AsyncReturnSteps {
    * A data-flow step for ordinary and exceptional returns from async functions.
    */
   private class AsyncReturn extends PreCallGraphStep {
-    // Note: partially copy-paste from FlowSteps::CachedSteps::returnStep/2
     override predicate storeStep(DataFlow::Node pred, DataFlow::SourceNode succ, string prop) {
       exists(DataFlow::FunctionNode f | f.getFunction().isAsync() |
         // ordinary return
@@ -478,6 +477,7 @@ private module AsyncReturnSteps {
         succ = f.getReturnNode()
         or
         // exceptional return
+        // Note: partially copy-paste from FlowSteps::localExceptionStep/2
         prop = errorProp() and
         exists(Expr expr |
           expr = any(ThrowStmt throw).getExpr() and
diff --git a/javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll b/javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll
index dd5e76c2d209..6d56c59288c1 100644
--- a/javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll
@@ -61,6 +61,7 @@ predicate localFlowStep(
  * Holds if an exception thrown from `pred` can propagate locally to `succ`.
  */
 predicate localExceptionStep(DataFlow::Node pred, DataFlow::Node succ) {
+  // Note: FlowSteps::localExceptionStep/2 has copy-paste children
   exists(Expr expr |
     expr = any(ThrowStmt throw).getExpr() and
     pred = expr.flow()
@@ -220,7 +221,6 @@ private module CachedSteps {
    */
   cached
   predicate returnStep(DataFlow::Node pred, DataFlow::Node succ) {
-    // Note: FlowSteps::CachedSteps::returnStep/2 has copy-paste children
     exists(Function f | calls(succ, f) or callsBound(succ, f, _) |
       DataFlow::functionReturnNode(pred, f)
       or

From 54fd7d97c07078f13c15509a6a76c2d12e60b6dc Mon Sep 17 00:00:00 2001
From: Erik Krogh Kristensen <erik-krogh@github.com>
Date: Fri, 7 Aug 2020 18:00:10 +0200
Subject: [PATCH 08/14] share implementation instead of copy-pasting

---
 javascript/ql/src/semmle/javascript/Promises.qll | 11 +----------
 .../javascript/dataflow/internal/FlowSteps.qll   | 16 +++++++++++++++-
 2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/javascript/ql/src/semmle/javascript/Promises.qll b/javascript/ql/src/semmle/javascript/Promises.qll
index 9a113e64e6bd..a893c1858289 100644
--- a/javascript/ql/src/semmle/javascript/Promises.qll
+++ b/javascript/ql/src/semmle/javascript/Promises.qll
@@ -477,17 +477,8 @@ private module AsyncReturnSteps {
         succ = f.getReturnNode()
         or
         // exceptional return
-        // Note: partially copy-paste from FlowSteps::localExceptionStep/2
         prop = errorProp() and
-        exists(Expr expr |
-          expr = any(ThrowStmt throw).getExpr() and
-          pred = expr.flow()
-          or
-          DataFlow::exceptionalInvocationReturnNode(pred, expr)
-        |
-          f.getFunction() = expr.getContainer() and
-          succ = f.getReturnNode() // returns a rejected promise - therefore using the ordinary return node.
-        )
+        localExceptionStepWithAsyncFlag(pred, succ, true)
       )
     }
   }
diff --git a/javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll b/javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll
index 6d56c59288c1..0282baca710e 100644
--- a/javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll
@@ -61,15 +61,29 @@ predicate localFlowStep(
  * Holds if an exception thrown from `pred` can propagate locally to `succ`.
  */
 predicate localExceptionStep(DataFlow::Node pred, DataFlow::Node succ) {
-  // Note: FlowSteps::localExceptionStep/2 has copy-paste children
+  localExceptionStepWithAsyncFlag(pred, succ, false)
+}
+
+/**
+ * Holds if an exception thrown from `pred` can propagate locally to `succ`.
+ *
+ * The `async` flag is true if the successor
+ */
+predicate localExceptionStepWithAsyncFlag(DataFlow::Node pred, DataFlow::Node succ, boolean async) {
   exists(Expr expr |
     expr = any(ThrowStmt throw).getExpr() and
     pred = expr.flow()
     or
     DataFlow::exceptionalInvocationReturnNode(pred, expr)
   |
+    async = false and
     succ = expr.getExceptionTarget() and
     not succ = any(DataFlow::FunctionNode f | f.getFunction().isAsync()).getExceptionalReturn()
+    or
+    async = true and
+    exists(DataFlow::FunctionNode f | f.getExceptionalReturn() = expr.getExceptionTarget() |
+      succ = f.getReturnNode() // returns a rejected promise - therefore using the ordinary return node.
+    )
   )
 }
 

From 2680afcdc9008f2b575dd6b3b4cadca8f2bad89f Mon Sep 17 00:00:00 2001
From: Erik Krogh Kristensen <erik-krogh@github.com>
Date: Fri, 7 Aug 2020 19:09:44 +0200
Subject: [PATCH 09/14] deduplicate some implementation in `storeStep` and
 `loadStep`

---
 .../javascript/dataflow/Configuration.qll     | 60 ++++++++-----------
 1 file changed, 26 insertions(+), 34 deletions(-)

diff --git a/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll b/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll
index 61378d86609e..13e237cda652 100644
--- a/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll
@@ -1019,10 +1019,16 @@ private predicate storeStep(
   isAdditionalStoreStep(pred, succ, prop, cfg) and
   summary = PathSummary::level()
   or
-  exists(Function f, DataFlow::Node mid | not f.isAsync() |
+  exists(Function f, DataFlow::Node mid, DataFlow::Node invk |
+    not f.isAsync() and invk = succ
+    or
+    // store in an immediately awaited function call
+    f.isAsync() and
+    invk = getAwaitOperand(succ)
+  |
     // `f` stores its parameter `pred` in property `prop` of a value that flows back to the caller,
     // and `succ` is an invocation of `f`
-    reachableFromInput(f, succ, pred, mid, cfg, summary) and
+    reachableFromInput(f, invk, pred, mid, cfg, summary) and
     (
       returnedPropWrite(f, _, prop, mid)
       or
@@ -1030,28 +1036,19 @@ private predicate storeStep(
         isAdditionalStoreStep(mid, base, prop, cfg)
       )
       or
-      succ instanceof DataFlow::NewNode and
+      invk instanceof DataFlow::NewNode and
       receiverPropWrite(f, prop, mid)
     )
   )
-  or
-  // store in an immediately awaited function call
-  exists(Function f, DataFlow::Node mid | f.isAsync() |
-    // `f` stores its parameter `pred` in property `prop` of a value that flows back to the caller,
-    // and `succ` is an invocation of `f`
-    exists(AwaitExpr await, DataFlow::Node operand |
-      operand = await.getOperand().getUnderlyingValue().flow() and
-      succ.asExpr() = await
-    |
-      reachableFromInput(f, operand, pred, mid, cfg, summary) and
-      (
-        returnedPropWrite(f, _, prop, mid)
-        or
-        exists(DataFlow::SourceNode base | base.flowsToExpr(f.getAReturnedExpr()) |
-          isAdditionalStoreStep(mid, base, prop, cfg)
-        )
-      )
-    )
+}
+
+/**
+ * Gets a dataflow-node for the operand of the await-expression `await`.
+ */
+private DataFlow::Node getAwaitOperand(DataFlow::Node await) {
+  exists(AwaitExpr awaitExpr |
+    result = awaitExpr.getOperand().getUnderlyingValue().flow() and
+    await.asExpr() = awaitExpr
   )
 }
 
@@ -1147,21 +1144,16 @@ private predicate loadStep(
   isAdditionalLoadStep(pred, succ, prop, cfg) and
   summary = PathSummary::level()
   or
-  exists(Function f, DataFlow::Node read | not f.isAsync() |
-    parameterPropRead(f, succ, pred, prop, read, cfg) and
+  exists(Function f, DataFlow::Node read, DataFlow::Node invk |
+    not f.isAsync() and invk = succ
+    or
+    // load from an immediately awaited function call
+    f.isAsync() and
+    invk = getAwaitOperand(succ)
+  |
+    parameterPropRead(f, invk, pred, prop, read, cfg) and
     reachesReturn(f, read, cfg, summary)
   )
-  or
-  // load from an immediately awaited function call
-  exists(Function f, DataFlow::Node read | f.isAsync() |
-    exists(AwaitExpr await, DataFlow::Node operand |
-      operand = await.getOperand().getUnderlyingValue().flow() and
-      succ.asExpr() = await
-    |
-      parameterPropRead(f, operand, pred, prop, read, cfg) and
-      reachesReturn(f, read, cfg, summary)
-    )
-  )
 }
 
 /**

From 244052f419aa797434fb780a4828134fa402a054 Mon Sep 17 00:00:00 2001
From: Erik Krogh Kristensen <erik-krogh@github.com>
Date: Sat, 8 Aug 2020 21:20:20 +0200
Subject: [PATCH 10/14] autoformat

---
 javascript/ql/src/semmle/javascript/dataflow/Nodes.qll | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll b/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll
index 81719228eedb..b8327265f702 100644
--- a/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll
@@ -494,7 +494,7 @@ class FunctionNode extends DataFlow::ValueNode, DataFlow::SourceNode {
 
   /**
    * Gets the data flow node representing the value returned from this function.
-   * 
+   *
    * Note that this differs from `getAReturn()`, in that every function has exactly
    * one canonical return node, but may have multiple (or zero) returned expressions.
    * The result of `getAReturn()` is always a predecessor of `getReturnNode()`

From 30dc77e5387dcaac0bc0d94f535f89f833f9cad2 Mon Sep 17 00:00:00 2001
From: Erik Krogh Kristensen <erik-krogh@github.com>
Date: Sat, 8 Aug 2020 21:26:45 +0200
Subject: [PATCH 11/14] update expected output

---
 .../test/library-tests/CallGraphs/FullTest/tests.expected   | 1 +
 .../library-tests/InterProceduralFlow/DataFlow.expected     | 6 +++---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/javascript/ql/test/library-tests/CallGraphs/FullTest/tests.expected b/javascript/ql/test/library-tests/CallGraphs/FullTest/tests.expected
index 90d47a022869..996dcecde532 100644
--- a/javascript/ql/test/library-tests/CallGraphs/FullTest/tests.expected
+++ b/javascript/ql/test/library-tests/CallGraphs/FullTest/tests.expected
@@ -67,6 +67,7 @@ test_getAFunctionValue
 | es2015.js:2:14:4:3 | () {\\n   ... ");\\n  } | es2015.js:2:14:4:3 | () {\\n   ... ");\\n  } |
 | es2015.js:6:5:6:16 | ExampleClass | es2015.js:2:14:4:3 | () {\\n   ... ");\\n  } |
 | es2015.js:8:2:12:1 | functio ... \\n  };\\n} | es2015.js:8:2:12:1 | functio ... \\n  };\\n} |
+| es2015.js:8:2:12:1 | return of anonymous function | es2015.js:9:10:11:3 | () => { ... ();\\n  } |
 | es2015.js:9:10:11:3 | () => { ... ();\\n  } | es2015.js:9:10:11:3 | () => { ... ();\\n  } |
 | es2015.js:10:5:10:20 | arguments.callee | es2015.js:8:2:12:1 | functio ... \\n  };\\n} |
 | es2015.js:10:5:10:22 | arguments.callee() | es2015.js:9:10:11:3 | () => { ... ();\\n  } |
diff --git a/javascript/ql/test/library-tests/InterProceduralFlow/DataFlow.expected b/javascript/ql/test/library-tests/InterProceduralFlow/DataFlow.expected
index c246c74b005d..664e6fada2b8 100644
--- a/javascript/ql/test/library-tests/InterProceduralFlow/DataFlow.expected
+++ b/javascript/ql/test/library-tests/InterProceduralFlow/DataFlow.expected
@@ -7,9 +7,9 @@
 | async.js:2:16:2:23 | "source" | async.js:36:17:36:17 | e |
 | async.js:2:16:2:23 | "source" | async.js:41:17:41:17 | e |
 | async.js:2:16:2:23 | "source" | async.js:54:17:54:36 | unpack(pack(source)) |
-| async.js:81:16:81:23 | "source" | async.js:82:14:82:36 | (await  ... ce))).p |
-| async.js:81:16:81:23 | "source" | async.js:94:15:94:30 | await (getP(o3)) |
-| async.js:98:18:98:25 | "source" | async.js:103:15:103:27 | await readP() |
+| async.js:79:16:79:23 | "source" | async.js:80:14:80:36 | (await  ... ce))).p |
+| async.js:79:16:79:23 | "source" | async.js:92:15:92:30 | await (getP(o3)) |
+| async.js:96:18:96:25 | "source" | async.js:101:15:101:27 | await readP() |
 | callback.js:16:14:16:21 | "source" | callback.js:13:14:13:14 | x |
 | callback.js:17:15:17:23 | "source2" | callback.js:13:14:13:14 | x |
 | callback.js:27:15:27:23 | "source3" | callback.js:13:14:13:14 | x |

From 9bcac10d9e03d42699f1108caa8737dc46f35c47 Mon Sep 17 00:00:00 2001
From: Erik Krogh Kristensen <erik-krogh@github.com>
Date: Mon, 10 Aug 2020 13:28:25 +0200
Subject: [PATCH 12/14] summarize exceptions thrown by immidiatly awaited
 function calls

---
 .../javascript/dataflow/Configuration.qll     | 11 ++++++++
 .../dataflow/internal/FlowSteps.qll           | 27 +++++++++++++------
 .../TaintTracking/BasicTaintTracking.expected |  1 +
 .../TaintTracking/DataFlowTracking.expected   |  1 +
 4 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll b/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll
index 13e237cda652..cd59f541a763 100644
--- a/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll
@@ -1000,6 +1000,17 @@ private predicate flowThroughCall(
     not isLabeledBarrierEdge(cfg, ret, output, summary.getEndLabel()) and
     not cfg.isLabeledBarrier(output, summary.getEndLabel())
   )
+  or
+  // exception thrown inside an immidiatly awaited function call.
+  exists(DataFlow::FunctionNode f, DataFlow::Node invk, DataFlow::Node ret |
+    f.getFunction().isAsync()
+  |
+    (calls(invk, f.getFunction()) or callsBound(invk, f.getFunction(), _)) and
+    reachableFromInput(f.getFunction(), invk, input, ret, cfg, summary) and
+    output = invk.asExpr().getExceptionTarget() and
+    f.getExceptionalReturn() = getThrowTarget(ret) and
+    invk = getAwaitOperand(_)
+  )
 }
 
 /**
diff --git a/javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll b/javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll
index 0282baca710e..099c77f1af66 100644
--- a/javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll
@@ -70,23 +70,34 @@ predicate localExceptionStep(DataFlow::Node pred, DataFlow::Node succ) {
  * The `async` flag is true if the successor
  */
 predicate localExceptionStepWithAsyncFlag(DataFlow::Node pred, DataFlow::Node succ, boolean async) {
-  exists(Expr expr |
-    expr = any(ThrowStmt throw).getExpr() and
-    pred = expr.flow()
-    or
-    DataFlow::exceptionalInvocationReturnNode(pred, expr)
-  |
+  exists(DataFlow::Node target | target = getThrowTarget(pred) |
     async = false and
-    succ = expr.getExceptionTarget() and
+    succ = target and
     not succ = any(DataFlow::FunctionNode f | f.getFunction().isAsync()).getExceptionalReturn()
     or
     async = true and
-    exists(DataFlow::FunctionNode f | f.getExceptionalReturn() = expr.getExceptionTarget() |
+    exists(DataFlow::FunctionNode f | f.getExceptionalReturn() = target |
       succ = f.getReturnNode() // returns a rejected promise - therefore using the ordinary return node.
     )
   )
 }
 
+/**
+ * Gets the dataflow-node that an exception thrown at `thrower` will flow to.
+ *
+ * The predicate that all functions are not async.
+ */
+DataFlow::Node getThrowTarget(DataFlow::Node thrower) {
+  exists(Expr expr |
+    expr = any(ThrowStmt throw).getExpr() and
+    thrower = expr.flow()
+    or
+    DataFlow::exceptionalInvocationReturnNode(thrower, expr)
+  |
+    result = expr.getExceptionTarget()
+  )
+}
+
 /**
  * Implements a set of data flow predicates that are used by multiple predicates and
  * hence should only be computed once.
diff --git a/javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected b/javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected
index bb717be7f9b5..823e8c350ac5 100644
--- a/javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected
+++ b/javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected
@@ -52,6 +52,7 @@ typeInferenceMismatch
 | exceptions.js:21:17:21:24 | source() | exceptions.js:24:10:24:21 | e.toString() |
 | exceptions.js:21:17:21:24 | source() | exceptions.js:25:10:25:18 | e.message |
 | exceptions.js:21:17:21:24 | source() | exceptions.js:26:10:26:19 | e.fileName |
+| exceptions.js:59:24:59:31 | source() | exceptions.js:61:12:61:12 | e |
 | exceptions.js:88:6:88:13 | source() | exceptions.js:11:10:11:10 | e |
 | exceptions.js:88:6:88:13 | source() | exceptions.js:32:10:32:10 | e |
 | exceptions.js:88:6:88:13 | source() | exceptions.js:33:10:33:21 | e.toString() |
diff --git a/javascript/ql/test/library-tests/TaintTracking/DataFlowTracking.expected b/javascript/ql/test/library-tests/TaintTracking/DataFlowTracking.expected
index f13257ba1395..a304345d522e 100644
--- a/javascript/ql/test/library-tests/TaintTracking/DataFlowTracking.expected
+++ b/javascript/ql/test/library-tests/TaintTracking/DataFlowTracking.expected
@@ -31,6 +31,7 @@
 | constructor-calls.js:14:15:14:22 | source() | constructor-calls.js:17:8:17:14 | c.param |
 | constructor-calls.js:14:15:14:22 | source() | constructor-calls.js:25:8:25:14 | d.param |
 | exceptions.js:3:15:3:22 | source() | exceptions.js:5:10:5:10 | e |
+| exceptions.js:59:24:59:31 | source() | exceptions.js:61:12:61:12 | e |
 | exceptions.js:88:6:88:13 | source() | exceptions.js:11:10:11:10 | e |
 | exceptions.js:93:11:93:18 | source() | exceptions.js:95:10:95:10 | e |
 | exceptions.js:100:13:100:20 | source() | exceptions.js:102:12:102:12 | e |

From 34778578dbfd29fb5b8e91b1f7e3051253590432 Mon Sep 17 00:00:00 2001
From: Erik Krogh Kristensen <erik-krogh@github.com>
Date: Mon, 10 Aug 2020 13:34:36 +0200
Subject: [PATCH 13/14] fill in docstring

---
 .../ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll    | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll b/javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll
index 099c77f1af66..57c7682963ab 100644
--- a/javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll
@@ -67,7 +67,7 @@ predicate localExceptionStep(DataFlow::Node pred, DataFlow::Node succ) {
 /**
  * Holds if an exception thrown from `pred` can propagate locally to `succ`.
  *
- * The `async` flag is true if the successor
+ * The `async` flag is true if the step involves wrapping the exception in a rejected Promise.
  */
 predicate localExceptionStepWithAsyncFlag(DataFlow::Node pred, DataFlow::Node succ, boolean async) {
   exists(DataFlow::Node target | target = getThrowTarget(pred) |

From e1ecc4662cc6c5857345e2c3bf42eab105e79c7c Mon Sep 17 00:00:00 2001
From: Erik Krogh Kristensen <erik-krogh@github.com>
Date: Tue, 11 Aug 2020 20:00:22 +0200
Subject: [PATCH 14/14] fix typo

Co-authored-by: Asger F <asgerf@github.com>
---
 javascript/ql/src/semmle/javascript/dataflow/Configuration.qll | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll b/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll
index cd59f541a763..8697d95a6b2d 100644
--- a/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll
@@ -1001,7 +1001,7 @@ private predicate flowThroughCall(
     not cfg.isLabeledBarrier(output, summary.getEndLabel())
   )
   or
-  // exception thrown inside an immidiatly awaited function call.
+  // exception thrown inside an immediately awaited function call.
   exists(DataFlow::FunctionNode f, DataFlow::Node invk, DataFlow::Node ret |
     f.getFunction().isAsync()
   |