Merge pull request github#6652 from asgerf/js/type-tracking-through-callback

Approved by erik-krogh

Author: codeql-ci

javascript/ql/lib/semmle/javascript/dataflow/Configuration.qll

@@ -947,7 +947,7 @@ private predicate exploratoryFlowStep(
   isAdditionalLoadStoreStep(pred, succ, _, _, cfg) or
   // the following three disjuncts taken together over-approximate flow through
   // higher-order calls
-  callback(pred, succ) or
+  exploratoryCallbackStep(pred, succ) or
   succ = pred.(DataFlow::FunctionNode).getAParameter() or
   exploratoryBoundInvokeStep(pred, succ)
 }

javascript/ql/lib/semmle/javascript/dataflow/TrackedNodes.qll

@@ -154,7 +154,7 @@ private module NodeTracking {
       or
       basicLoadStep(mid, nd, _)
       or
-      callback(mid, nd)
+      exploratoryCallbackStep(mid, nd)
       or
       nd = mid.(DataFlow::FunctionNode).getAParameter()
     )

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

@@ -434,7 +434,7 @@ private module CachedSteps {
    * invocation.
    */
   cached
-  predicate callback(DataFlow::Node arg, DataFlow::SourceNode cb) {
+  predicate exploratoryCallbackStep(DataFlow::Node arg, DataFlow::SourceNode cb) {
     Stages::TypeTracking::ref() and
     exists(DataFlow::InvokeNode invk, DataFlow::ParameterNode cbParm, DataFlow::Node cbArg |
       arg = invk.getAnArgument() and
@@ -444,12 +444,24 @@ private module CachedSteps {
     )
     or
     exists(DataFlow::ParameterNode cbParm, DataFlow::Node cbArg |
-      callback(arg, cbParm) and
+      exploratoryCallbackStep(arg, cbParm) and
       callStep(cbArg, cbParm) and
       cb.flowsTo(cbArg)
     )
   }
 
+  /** Gets a function that flows to `parameter` via one or more parameter-passing steps. */
+  cached
+  DataFlow::FunctionNode getACallbackSource(DataFlow::ParameterNode parameter) {
+    Stages::TypeTracking::ref() and
+    callStep(result.getALocalUse(), parameter)
+    or
+    exists(DataFlow::ParameterNode mid |
+      callStep(mid.getALocalUse(), parameter) and
+      result = getACallbackSource(mid)
+    )
+  }
+
   /**
    * Holds if `f` may return `base`, which has a write of property `prop` with right-hand side `rhs`.
    */

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

@@ -156,6 +156,13 @@ private module Cached {
         succ = fun.getAnInvocation()
       )
     )
+    or
+    // Add 'return' steps from callback arguments to callback parameters
+    exists(DataFlow::ParameterNode parameter, int i |
+      pred = parameter.getAnInvocation().getArgument(i) and
+      succ = getACallbackSource(parameter).getParameter(i) and
+      summary = ReturnStep()
+    )
   }
 }
 

javascript/ql/test/library-tests/TypeTracking/ClassStyle.expected

@@ -38,6 +38,9 @@ test_Connection
 | tst.js:112:10:112:14 | obj.x |
 | tst.js:114:1:114:28 | getX({  ... on() }) |
 | tst.js:114:11:114:25 | getConnection() |
+| tst.js:118:12:118:26 | getConnection() |
+| tst.js:120:21:120:24 | conn |
+| tst.js:126:22:126:25 | conn |
 | tst_conflict.js:6:38:6:77 | api.cha ... ction() |
 test_DataCallback
 | client.js:3:28:3:34 | x => {} |

javascript/ql/test/library-tests/TypeTracking/PredicateStyle.expected

@@ -40,6 +40,9 @@ connection
 | type tracker without call steps | tst.js:108:8:108:22 | getConnection() |
 | type tracker without call steps | tst.js:114:1:114:28 | getX({  ... on() }) |
 | type tracker without call steps | tst.js:114:11:114:25 | getConnection() |
+| type tracker without call steps | tst.js:118:12:118:26 | getConnection() |
+| type tracker without call steps | tst.js:120:21:120:24 | conn |
+| type tracker without call steps | tst.js:126:22:126:25 | conn |
 | type tracker without call steps | tst_conflict.js:6:38:6:77 | api.cha ... ction() |
 | type tracker without call steps with property MyApplication.namespace.connection | file://:0:0:0:0 | global access path |
 | type tracker without call steps with property conflict | tst.js:63:3:63:25 | MyAppli ... mespace |

javascript/ql/test/library-tests/TypeTracking/tst.js

@@ -113,3 +113,15 @@ function getX(obj) {
 }
 getX({ x: getConnection() });
 getX({ x: somethingElse() });
+
+function makeConnectionAsync(callback) {
+  callback(getConnection());
+}
+makeConnectionAsync(conn => {});
+makeConnectionAsync(); // suppress single-call precision gains
+
+function makeConnectionAsync2(callback) {
+  makeConnectionAsync(callback);
+}
+makeConnectionAsync2(conn => {});
+makeConnectionAsync2(); // suppress single-call precision gains