JavaScript: Only follow level flow steps when summarising functions.

It is not only wasteful to consider paths with unmatched calls/returns,
but also wrong; see test case in next commit.

Author: Max Schaefer

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

@@ -557,7 +557,8 @@ private predicate callInputStep(
 /**
  * Holds if `input`, which is either an argument to `f` at `invk` or a definition
  * that is captured by `f`, may flow to `nd` under configuration `cfg` (possibly through
- * callees) along a path summarized by `summary`.
+ * callees, but not containing any unmatched calls or returns) along a path summarized by
+ * `summary`.
  *
  * Note that the summary does not take the initial step from argument to parameter
  * into account.
@@ -577,7 +578,7 @@ private predicate reachableFromInput(
 }
 
 /**
- * Holds if there is a step from `pred` to `succ` under `cfg` that can be appended
+ * Holds if there is a level step from `pred` to `succ` under `cfg` that can be appended
  * to a path represented by `oldSummary` yielding a path represented by `newSummary`.
  */
 pragma[noinline]
@@ -587,6 +588,7 @@ private predicate appendStep(
 ) {
   exists(PathSummary stepSummary |
     flowStep(pred, cfg, succ, stepSummary) and
+    stepSummary.isLevel() and
     newSummary = oldSummary.append(stepSummary)
   )
 }

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

@@ -131,8 +131,8 @@ private module NodeTracking {
 
   /**
    * Holds if `input`, which is either an argument to `f` at `invk` or a definition
-   * that is captured by `f`, may flow to `nd` (possibly through callees) along
-   * a path summarized by `summary`.
+   * that is captured by `f`, may flow to `nd` (possibly through callees, but not containing
+   * any unmatched calls or returns) along a path summarized by `summary`.
    */
   private predicate reachableFromInput(
     Function f, DataFlow::Node invk, DataFlow::Node input, DataFlow::Node nd, PathSummary summary
@@ -143,6 +143,7 @@ private module NodeTracking {
     exists(DataFlow::Node mid, PathSummary oldSummary, PathSummary newSummary |
       reachableFromInput(f, invk, input, mid, oldSummary) and
       flowStep(mid, nd, newSummary) and
+      newSummary.isLevel() and
       summary = oldSummary.append(newSummary)
     )
   }

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

@@ -340,12 +340,17 @@ class PathSummary extends TPathSummary {
 
   PathSummary() { this = MkPathSummary(hasReturn, hasCall, start, end) }
 
-  /** Indicates whether the path represented by this summary contains any return steps. */
+  /** Indicates whether the path represented by this summary contains any unmatched return steps. */
   boolean hasReturn() { result = hasReturn }
 
-  /** Indicates whether the path represented by this summary contains any call steps. */
+  /** Indicates whether the path represented by this summary contains any unmatched call steps. */
   boolean hasCall() { result = hasCall }
 
+  /** Holds if the path represented by this summary contains no unmatched call or return steps. */
+  predicate isLevel() {
+    hasReturn = false and hasCall = false
+  }
+
   /** Gets the flow label describing the value at the start of this flow path. */
   FlowLabel getStartLabel() { result = start }