JavaScript: Improve terminology and comments.

Author: Max Schaefer

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

@@ -442,7 +442,9 @@ private predicate exploratoryFlowStep(
   basicFlowStep(pred, succ, _, cfg) or
   basicStoreStep(pred, succ, _) or
   loadStep(pred, succ, _) or
-  approximateCallbackStep(pred, succ) or
+  // the following two disjuncts taken together over-approximate flow through
+  // higher-order calls
+  callback(pred, succ) or
   succ = pred.(DataFlow::FunctionNode).getAParameter()
 }
 
@@ -622,9 +624,10 @@ private predicate flowThroughProperty(
 
 /**
  * Holds if `arg` and `cb` are passed as arguments to a function which in turn
- * invokes `cb`, passing `arg` as its `i`th argument. All of this is done under
- * configuration `cfg`, and `arg` flows along a path summarized by `summary`,
- * while `cb` is only tracked locally.
+ * invokes `cb`, passing `arg` as its `i`th argument.
+ *
+ * All of this is done under configuration `cfg`, and `arg` flows along a path
+ * summarized by `summary`, while `cb` is only tracked locally.
  */
 private predicate higherOrderCall(
   DataFlow::Node arg, DataFlow::Node cb, int i, DataFlow::Configuration cfg, PathSummary summary
@@ -634,10 +637,12 @@ private predicate higherOrderCall(
     reachableFromInput(f, outer, arg, innerArg, cfg, oldSummary) and
     argumentPassing(outer, cb, f, cbParm) and
     innerArg = inner.getArgument(j) |
+    // direct higher-order call
     cbParm.flowsTo(inner.getCalleeNode()) and
     i = j and
     summary = oldSummary
     or
+    // indirect higher-order call
     exists (DataFlow::Node cbArg, PathSummary newSummary |
       cbParm.flowsTo(cbArg) and
       higherOrderCall(innerArg, cbArg, i, cfg, newSummary) and
@@ -649,7 +654,9 @@ private predicate higherOrderCall(
 /**
  * Holds if `pred` is passed as an argument to a function `f` which also takes a
  * callback parameter `cb` and then invokes `cb`, passing `pred` into parameter `succ`
- * of `cb`. All of this is done under configuration `cfg`, and `arg` flows along a path
+ * of `cb`.
+ *
+ * All of this is done under configuration `cfg`, and `arg` flows along a path
  * summarized by `summary`, while `cb` is only tracked locally.
  */
 private predicate flowIntoHigherOrderCall(

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

@@ -101,7 +101,7 @@ private module NodeTracking {
       or
       loadStep(mid, nd, _)
       or
-      approximateCallbackStep(mid, nd)
+      callback(mid, nd)
       or
       nd = mid.(DataFlow::FunctionNode).getAParameter()
     )
@@ -220,10 +220,12 @@ private module NodeTracking {
       reachableFromInput(f, outer, arg, innerArg, oldSummary) and
       argumentPassing(outer, cb, f, cbParm) and
       innerArg = inner.getArgument(j) |
+      // direct higher-order call
       cbParm.flowsTo(inner.getCalleeNode()) and
       i = j and
       summary = oldSummary
       or
+      // indirect higher-order call
       exists (DataFlow::Node cbArg, PathSummary newSummary |
         cbParm.flowsTo(cbArg) and
         higherOrderCall(innerArg, cbArg, i, newSummary) and

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

@@ -236,17 +236,30 @@ predicate loadStep(DataFlow::Node pred, DataFlow::PropRead succ, string prop) {
 }
 
 /**
- * Holds if there is a call with argument `pred`, and `succ` flows into the callee
- * position of that call.
+ * Holds if there is a higher-order call with argument `arg`, and `cb` is the local
+ * source of an argument that flows into the callee position of that call:
+ *
+ * ```
+ * function f(x, g) {
+ *   g(
+ *     x                 // arg
+ *   );
+ * }
+ *
+ * function cb() {      // cb
+ * }
+ *
+ * f(arg, cb);
  *
  * This is an over-approximation of a possible data flow step through a callback
  * invocation.
  */
-predicate approximateCallbackStep(DataFlow::Node pred, DataFlow::SourceNode succ) {
-  exists (DataFlow::InvokeNode invk, DataFlow::ParameterNode cb |
-    pred = invk.getAnArgument() and
-    cb.flowsTo(invk.getCalleeNode()) and
-    callStep(any(DataFlow::Node nd | succ.flowsTo(nd)), cb)
+predicate callback(DataFlow::Node arg, DataFlow::SourceNode cb) {
+  exists (DataFlow::InvokeNode invk, DataFlow::ParameterNode cbParm, DataFlow::Node cbArg |
+    arg = invk.getAnArgument() and
+    cbParm.flowsTo(invk.getCalleeNode()) and
+    callStep(cbArg, cbParm) and
+    cb.flowsTo(cbArg)
   )
 }