Merge pull request #3036 from erik-krogh/CustomTrack

Approved by asgerf

javascript/ql/src/semmle/javascript/Promises.qll

@@ -3,6 +3,7 @@
  */
 
 import javascript
+private import dataflow.internal.StepSummary
 
 /**
  * A definition of a `Promise` object.
@@ -121,36 +122,156 @@ class AggregateES2015PromiseDefinition extends PromiseCreationCall {
 }
 
 /**
- * This module defines how data-flow propagates into and out of a Promise.
+ * Common predicates shared between type-tracking and data-flow for promises.
- * The data-flow is based on pseudo-properties rather than tainting the Promise object (which is what `PromiseTaintStep` does).
  */
-private module PromiseFlow {
+module Promises {
   /**
    * Gets the pseudo-field used to describe resolved values in a promise.
    */
-  string resolveField() { result = "$PromiseResolveField$" }
+  string valueProp() { result = "$PromiseResolveField$" }
 
   /**
    * Gets the pseudo-field used to describe rejected values in a promise.
    */
-  string rejectField() { result = "$PromiseRejectField$" }
+  string errorProp() { result = "$PromiseRejectField$" }
+}
+
+/**
+ * A module for supporting promises in type-tracking predicates.
+ * The `PromiseTypeTracking::promiseStep` predicate is used for type tracking in and out of promises,
+ * and is included in the standard type-tracking steps (`SourceNode::track`).
+ * The `TypeTracker::startInPromise()` predicate can be used to initiate a type-tracker
+ * where the tracked value is a promise.
+ *
+ * The below is an example of a type-tracking predicate where the initial value is a promise:
+ * ```
+ * DataFlow::SourceNode myType(DataFlow::TypeTracker t) {
+ *  t.startInPromise() and
+ *  result = <the promise value> and
+ *  or
+ *  exists(DataFlow::TypeTracker t2 | result = myType(t2).track(t2, t))
+ * }
+ * ```
+ *
+ * The type-tracking predicate above will only end (`t = DataFlow::TypeTracker::end()`) after the tracked value has been
+ * extracted from the promise.
+ *
+ * The `PromiseTypeTracking::promiseStep` predicate can be used instead of `SourceNode::track`
+ * to get type-tracking only for promise steps.
+ *
+ * Replace `t.startInPromise()` in the above example with `t.start()` to create a type-tracking predicate
+ * where the value is not initially inside a promise.
+ */
+module PromiseTypeTracking {
+  /**
+   * Gets the result from a single step through a promise, from `pred` to `result` summarized by `summary`.
+   * This can be loading a resolved value from a promise, storing a value in a promise, or copying a resolved value from one promise to another.
+   */
+  DataFlow::SourceNode promiseStep(DataFlow::SourceNode pred, StepSummary summary) {
+    exists(PromiseFlowStep step, string field | field = Promises::valueProp() |
+      summary = LoadStep(field) and
+      step.load(pred, result, field)
+      or
+      summary = StoreStep(field) and
+      step.store(pred, result, field)
+      or
+      summary = LevelStep() and
+      step.loadStore(pred, result, field)
+    )
+  }
+
+  /**
+   * Gets the result from a single step through a promise, from `pred` with tracker `t2` to `result` with tracker `t`.
+   * This can be loading a resolved value from a promise, storing a value in a promise, or copying a resolved value from one promise to another.
+   */
+  DataFlow::SourceNode promiseStep(
+    DataFlow::SourceNode pred, DataFlow::TypeTracker t, DataFlow::TypeTracker t2
+  ) {
+    exists(StepSummary summary |
+      result = PromiseTypeTracking::promiseStep(pred, summary) and
+      t = t2.append(summary)
+    )
+  }
+
+  /**
+   * A class enabling the use of the `resolveField` as a pseudo-property in type-tracking predicates.
+   */
+  private class ResolveFieldAsTypeTrackingProperty extends TypeTrackingPseudoProperty {
+    ResolveFieldAsTypeTrackingProperty() { this = Promises::valueProp() }
+  }
+}
+
+/**
+ * An `AdditionalFlowStep` used to model a data-flow step related to promises.
+ *
+ * The `loadStep`/`storeStep`/`loadStoreStep` methods are overloaded such that the new predicates
+ * `load`/`store`/`loadStore` can be used in the `PromiseTypeTracking` module.
+ * (Thereby avoiding conflicts with a "cousin" `AdditionalFlowStep` implementation.)
+ *
+ * The class is private and is only intended to be used inside the `PromiseTypeTracking` and `PromiseFlow` modules.
+ */
+abstract private class PromiseFlowStep extends DataFlow::AdditionalFlowStep {
+  final override predicate step(DataFlow::Node pred, DataFlow::Node succ) { none() }
+
+  final override predicate step(
+    DataFlow::Node p, DataFlow::Node s, DataFlow::FlowLabel pl, DataFlow::FlowLabel sl
+  ) {
+    none()
+  }
+
+  /**
+   * Holds if the property `prop` of the object `pred` should be loaded into `succ`.
+   */
+  predicate load(DataFlow::Node pred, DataFlow::Node succ, string prop) { none() }
+
+  final override predicate loadStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
+    this.load(pred, succ, prop)
+  }
+
+  /**
+   * Holds if `pred` should be stored in the object `succ` under the property `prop`.
+   */
+  predicate store(DataFlow::Node pred, DataFlow::Node succ, string prop) { none() }
+
+  final override predicate storeStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
+    this.store(pred, succ, prop)
+  }
+
+  /**
+   * Holds if the property `prop` should be copied from the object `pred` to the object `succ`.
+   */
+  predicate loadStore(DataFlow::Node pred, DataFlow::Node succ, string prop) { none() }
+
+  final override predicate loadStoreStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
+    this.loadStore(pred, succ, prop)
+  }
+}
+
+/**
+ * This module defines how data-flow propagates into and out of a Promise.
+ * The data-flow is based on pseudo-properties rather than tainting the Promise object (which is what `PromiseTaintStep` does).
+ */
+private module PromiseFlow {
+  private predicate valueProp = Promises::valueProp/0;
+
+  private predicate errorProp = Promises::errorProp/0;
 
   /**
    * A flow step describing a promise definition.
    *
    * The resolved/rejected value is written to a pseudo-field on the promise.
    */
-  class PromiseDefitionStep extends DataFlow::AdditionalFlowStep {
+  class PromiseDefitionStep extends PromiseFlowStep {
     PromiseDefinition promise;
 
     PromiseDefitionStep() { this = promise }
 
-    override predicate storeStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
+    override predicate store(DataFlow::Node pred, DataFlow::Node succ, string prop) {
-      prop = resolveField() and
+      prop = valueProp() and
       pred = promise.getResolveParameter().getACall().getArgument(0) and
       succ = this
       or
-      prop = rejectField() and
+      prop = errorProp() and
       (
         pred = promise.getRejectParameter().getACall().getArgument(0) or
         pred = promise.getExecutor().getExceptionalReturn()
@@ -158,9 +279,9 @@ private module PromiseFlow {
       succ = this
     }
 
-    override predicate loadStoreStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
+    override predicate loadStore(DataFlow::Node pred, DataFlow::Node succ, string prop) {
       // Copy the value of a resolved promise to the value of this promise.
-      prop = resolveField() and
+      prop = valueProp() and
       pred = promise.getResolveParameter().getACall().getArgument(0) and
       succ = this
     }
@@ -169,20 +290,20 @@ private module PromiseFlow {
   /**
    * A flow step describing the a Promise.resolve (and similar) call.
    */
-  class CreationStep extends DataFlow::AdditionalFlowStep {
+  class CreationStep extends PromiseFlowStep {
     PromiseCreationCall promise;
 
     CreationStep() { this = promise }
 
-    override predicate storeStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
+    override predicate store(DataFlow::Node pred, DataFlow::Node succ, string prop) {
-      prop = resolveField() and
+      prop = valueProp() and
       pred = promise.getValue() and
       succ = this
     }
 
-    override predicate loadStoreStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
+    override predicate loadStore(DataFlow::Node pred, DataFlow::Node succ, string prop) {
       // Copy the value of a resolved promise to the value of this promise.
-      prop = resolveField() and
+      prop = valueProp() and
       pred = promise.getValue() and
       succ = this
     }
@@ -192,7 +313,7 @@ private module PromiseFlow {
    * A load step loading the pseudo-field describing that the promise is rejected.
    * The rejected value is thrown as a exception.
    */
-  class AwaitStep extends DataFlow::AdditionalFlowStep {
+  class AwaitStep extends PromiseFlowStep {
     DataFlow::Node operand;
     AwaitExpr await;
 
@@ -201,12 +322,12 @@ private module PromiseFlow {
       operand.getEnclosingExpr() = await.getOperand()
     }
 
-    override predicate loadStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
+    override predicate load(DataFlow::Node pred, DataFlow::Node succ, string prop) {
-      prop = resolveField() and
+      prop = valueProp() and
       succ = this and
       pred = operand
       or
-      prop = rejectField() and
+      prop = errorProp() and
       succ = await.getExceptionTarget() and
       pred = operand
     }
@@ -215,37 +336,37 @@ private module PromiseFlow {
   /**
    * A flow step describing the data-flow related to the `.then` method of a promise.
    */
-  class ThenStep extends DataFlow::AdditionalFlowStep, DataFlow::MethodCallNode {
+  class ThenStep extends PromiseFlowStep, DataFlow::MethodCallNode {
     ThenStep() { this.getMethodName() = "then" }
 
-    override predicate loadStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
+    override predicate load(DataFlow::Node pred, DataFlow::Node succ, string prop) {
-      prop = resolveField() and
+      prop = valueProp() and
       pred = getReceiver() and
       succ = getCallback(0).getParameter(0)
       or
-      prop = rejectField() and
+      prop = errorProp() and
       pred = getReceiver() and
       succ = getCallback(1).getParameter(0)
     }
 
-    override predicate loadStoreStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
+    override predicate loadStore(DataFlow::Node pred, DataFlow::Node succ, string prop) {
       not exists(this.getArgument(1)) and
-      prop = rejectField() and
+      prop = errorProp() and
       pred = getReceiver() and
       succ = this
       or
       // read the value of a resolved/rejected promise that is returned
-      (prop = rejectField() or prop = resolveField()) and
+      (prop = errorProp() or prop = valueProp()) and
       pred = getCallback([0 .. 1]).getAReturn() and
       succ = this
     }
 
-    override predicate storeStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
+    override predicate store(DataFlow::Node pred, DataFlow::Node succ, string prop) {
-      prop = resolveField() and
+      prop = valueProp() and
       pred = getCallback([0 .. 1]).getAReturn() and
       succ = this
       or
-      prop = rejectField() and
+      prop = errorProp() and
       pred = getCallback([0 .. 1]).getExceptionalReturn() and
       succ = this
     }
@@ -254,32 +375,32 @@ private module PromiseFlow {
   /**
    * A flow step describing the data-flow related to the `.catch` method of a promise.
    */
-  class CatchStep extends DataFlow::AdditionalFlowStep, DataFlow::MethodCallNode {
+  class CatchStep extends PromiseFlowStep, DataFlow::MethodCallNode {
     CatchStep() { this.getMethodName() = "catch" }
 
-    override predicate loadStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
+    override predicate load(DataFlow::Node pred, DataFlow::Node succ, string prop) {
-      prop = rejectField() and
+      prop = errorProp() and
       pred = getReceiver() and
       succ = getCallback(0).getParameter(0)
     }
 
-    override predicate loadStoreStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
+    override predicate loadStore(DataFlow::Node pred, DataFlow::Node succ, string prop) {
-      prop = resolveField() and
+      prop = valueProp() and
       pred = getReceiver().getALocalSource() and
       succ = this
       or
       // read the value of a resolved/rejected promise that is returned
-      (prop = rejectField() or prop = resolveField()) and
+      (prop = errorProp() or prop = valueProp()) and
       pred = getCallback(0).getAReturn() and
       succ = this
     }
 
-    override predicate storeStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
+    override predicate store(DataFlow::Node pred, DataFlow::Node succ, string prop) {
-      prop = rejectField() and
+      prop = errorProp() and
       pred = getCallback(0).getExceptionalReturn() and
       succ = this
       or
-      prop = resolveField() and
+      prop = valueProp() and
       pred = getCallback(0).getAReturn() and
       succ = this
     }
@@ -288,22 +409,22 @@ private module PromiseFlow {
   /**
    * A flow step describing the data-flow related to the `.finally` method of a promise.
    */
-  class FinallyStep extends DataFlow::AdditionalFlowStep, DataFlow::MethodCallNode {
+  class FinallyStep extends PromiseFlowStep, DataFlow::MethodCallNode {
     FinallyStep() { this.getMethodName() = "finally" }
 
-    override predicate loadStoreStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
+    override predicate loadStore(DataFlow::Node pred, DataFlow::Node succ, string prop) {
-      (prop = resolveField() or prop = rejectField()) and
+      (prop = valueProp() or prop = errorProp()) and
       pred = getReceiver() and
       succ = this
       or
       // read the value of a rejected promise that is returned
-      prop = rejectField() and
+      prop = errorProp() and
       pred = getCallback(0).getAReturn() and
       succ = this
     }
 
-    override predicate storeStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
+    override predicate store(DataFlow::Node pred, DataFlow::Node succ, string prop) {
-      prop = rejectField() and
+      prop = errorProp() and
       pred = getCallback(0).getExceptionalReturn() and
       succ = this
     }

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

@@ -8,147 +8,7 @@
 
 private import javascript
 private import internal.FlowSteps
-
+private import internal.StepSummary
-private class PropertyName extends string {
-  PropertyName() {
-    this = any(DataFlow::PropRef pr).getPropertyName()
-    or
-    AccessPath::isAssignedInUniqueFile(this)
-    or
-    exists(AccessPath::getAnAssignmentTo(_, this))
-  }
-}
-
-private class OptionalPropertyName extends string {
-  OptionalPropertyName() { this instanceof PropertyName or this = "" }
-}
-
-/**
- * A description of a step on an inter-procedural data flow path.
- */
-private newtype TStepSummary =
-  LevelStep() or
-  CallStep() or
-  ReturnStep() or
-  StoreStep(PropertyName prop) or
-  LoadStep(PropertyName prop)
-
-/**
- * INTERNAL: Use `TypeTracker` or `TypeBackTracker` instead.
- *
- * A description of a step on an inter-procedural data flow path.
- */
-class StepSummary extends TStepSummary {
-  /** Gets a textual representation of this step summary. */
-  string toString() {
-    this instanceof LevelStep and result = "level"
-    or
-    this instanceof CallStep and result = "call"
-    or
-    this instanceof ReturnStep and result = "return"
-    or
-    exists(string prop | this = StoreStep(prop) | result = "store " + prop)
-    or
-    exists(string prop | this = LoadStep(prop) | result = "load " + prop)
-  }
-}
-
-module StepSummary {
-  /**
-   * INTERNAL: Use `SourceNode.track()` or `SourceNode.backtrack()` instead.
-   */
-  cached
-  predicate step(DataFlow::SourceNode pred, DataFlow::SourceNode succ, StepSummary summary) {
-    exists(DataFlow::Node mid | pred.flowsTo(mid) | smallstep(mid, succ, summary))
-  }
-
-  /**
-   * INTERNAL: Use `TypeBackTracker.smallstep()` instead.
-   */
-  predicate smallstep(DataFlow::Node pred, DataFlow::Node succ, StepSummary summary) {
-    // Flow through properties of objects
-    propertyFlowStep(pred, succ) and
-    summary = LevelStep()
-    or
-    // Flow through global variables
-    globalFlowStep(pred, succ) and
-    summary = LevelStep()
-    or
-    // Flow into function
-    callStep(pred, succ) and
-    summary = CallStep()
-    or
-    // Flow out of function
-    returnStep(pred, succ) and
-    summary = ReturnStep()
-    or
-    // Flow through an instance field between members of the same class
-    DataFlow::localFieldStep(pred, succ) and
-    summary = LevelStep()
-    or
-    exists(string prop |
-      basicStoreStep(pred, succ, prop) and
-      summary = StoreStep(prop)
-      or
-      basicLoadStep(pred, succ, prop) and
-      summary = LoadStep(prop)
-    )
-    or
-    any(AdditionalTypeTrackingStep st).step(pred, succ) and
-    summary = LevelStep()
-    or
-    // Store to global access path
-    exists(string name |
-      pred = AccessPath::getAnAssignmentTo(name) and
-      AccessPath::isAssignedInUniqueFile(name) and
-      succ = DataFlow::globalAccessPathRootPseudoNode() and
-      summary = StoreStep(name)
-    )
-    or
-    // Load from global access path
-    exists(string name |
-      succ = AccessPath::getAReferenceTo(name) and
-      AccessPath::isAssignedInUniqueFile(name) and
-      pred = DataFlow::globalAccessPathRootPseudoNode() and
-      summary = LoadStep(name)
-    )
-    or
-    // Store to non-global access path
-    exists(string name |
-      pred = AccessPath::getAnAssignmentTo(succ, name) and
-      summary = StoreStep(name)
-    )
-    or
-    // Load from non-global access path
-    exists(string name |
-      succ = AccessPath::getAReferenceTo(pred, name) and
-      summary = LoadStep(name) and
-      name != ""
-    )
-    or
-    // Summarize calls with flow directly from a parameter to a return.
-    exists(DataFlow::ParameterNode param, DataFlow::FunctionNode fun |
-      (
-        param.flowsTo(fun.getAReturn()) and
-        summary = LevelStep()
-        or
-        exists(string prop |
-          param.getAPropertyRead(prop).flowsTo(fun.getAReturn()) and
-          summary = LoadStep(prop)
-        )
-      ) and
-      if param = fun.getAParameter()
-      then
-        // Step from argument to call site.
-        argumentPassing(succ, pred, fun.getFunction(), param)
-      else (
-        // Step from captured parameter to local call sites
-        pred = param and
-        succ = fun.getAnInvocation()
-      )
-    )
-  }
-}
 
 private newtype TTypeTracker = MkTypeTracker(Boolean hasCall, OptionalPropertyName prop)
 
@@ -216,6 +76,18 @@ class TypeTracker extends TTypeTracker {
    */
   predicate start() { hasCall = false and prop = "" }
 
+  /**
+   * Holds if this is the starting point of type tracking, and the value starts in the property named `propName`.
+   * The type tracking only ends after the property has been loaded.
+   */
+  predicate startInProp(PropertyName propName) { hasCall = false and prop = propName }
+
+  /**
+   * Holds if this is the starting point of type tracking, and the initial value is a promise.
+   * The type tracking only ends after the value has been extracted from the promise.
+   */
+  predicate startInPromise() { startInProp(Promises::valueProp()) }
+
   /**
    * Holds if this is the starting point of type tracking
    * when tracking a parameter into a call, but not out of it.

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

@@ -3,6 +3,7 @@
  */
 
 private import javascript
+private import semmle.javascript.dataflow.internal.StepSummary
 
 cached
 module CallGraph {
@@ -83,7 +84,7 @@ module CallGraph {
       getAFunctionReference(function, 0, t.continue()).flowsTo(callback)
     )
     or
-    exists(DataFlow::StepSummary summary, DataFlow::TypeTracker t2 |
+    exists(StepSummary summary, DataFlow::TypeTracker t2 |
       result = getABoundFunctionReferenceAux(function, boundArgs, t2, summary) and
       t = t2.append(summary)
     )
@@ -91,12 +92,11 @@ module CallGraph {
 
   pragma[noinline]
   private DataFlow::SourceNode getABoundFunctionReferenceAux(
-    DataFlow::FunctionNode function, int boundArgs, DataFlow::TypeTracker t,
+    DataFlow::FunctionNode function, int boundArgs, DataFlow::TypeTracker t, StepSummary summary
-    DataFlow::StepSummary summary
   ) {
     exists(DataFlow::SourceNode prev |
       prev = getABoundFunctionReferenceAux(function, boundArgs, t) and
-      DataFlow::StepSummary::step(prev, result, summary)
+      StepSummary::step(prev, result, summary)
     )
   }
 

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

@@ -0,0 +1,157 @@
+import javascript
+private import semmle.javascript.dataflow.TypeTracking
+private import FlowSteps
+
+class PropertyName extends string {
+  PropertyName() {
+    this = any(DataFlow::PropRef pr).getPropertyName()
+    or
+    AccessPath::isAssignedInUniqueFile(this)
+    or
+    exists(AccessPath::getAnAssignmentTo(_, this))
+    or
+    this instanceof TypeTrackingPseudoProperty
+  }
+}
+
+class OptionalPropertyName extends string {
+  OptionalPropertyName() { this instanceof PropertyName or this = "" }
+}
+
+/**
+ * A pseudo-property that can be used in type-tracking.
+ */
+abstract class TypeTrackingPseudoProperty extends string {
+  bindingset[this]
+  TypeTrackingPseudoProperty() { any() }
+}
+
+/**
+ * A description of a step on an inter-procedural data flow path.
+ */
+newtype TStepSummary =
+  LevelStep() or
+  CallStep() or
+  ReturnStep() or
+  StoreStep(PropertyName prop) or
+  LoadStep(PropertyName prop)
+
+/**
+ * INTERNAL: Use `TypeTracker` or `TypeBackTracker` instead.
+ *
+ * A description of a step on an inter-procedural data flow path.
+ */
+class StepSummary extends TStepSummary {
+  /** Gets a textual representation of this step summary. */
+  string toString() {
+    this instanceof LevelStep and result = "level"
+    or
+    this instanceof CallStep and result = "call"
+    or
+    this instanceof ReturnStep and result = "return"
+    or
+    exists(string prop | this = StoreStep(prop) | result = "store " + prop)
+    or
+    exists(string prop | this = LoadStep(prop) | result = "load " + prop)
+  }
+}
+
+module StepSummary {
+  /**
+   * INTERNAL: Use `SourceNode.track()` or `SourceNode.backtrack()` instead.
+   */
+  cached
+  predicate step(DataFlow::SourceNode pred, DataFlow::SourceNode succ, StepSummary summary) {
+    exists(DataFlow::Node mid | pred.flowsTo(mid) | smallstep(mid, succ, summary))
+  }
+
+  /**
+   * INTERNAL: Use `TypeBackTracker.smallstep()` instead.
+   */
+  predicate smallstep(DataFlow::Node pred, DataFlow::Node succ, StepSummary summary) {
+    // Flow through properties of objects
+    propertyFlowStep(pred, succ) and
+    summary = LevelStep()
+    or
+    // Flow through global variables
+    globalFlowStep(pred, succ) and
+    summary = LevelStep()
+    or
+    // Flow into function
+    callStep(pred, succ) and
+    summary = CallStep()
+    or
+    // Flow out of function
+    returnStep(pred, succ) and
+    summary = ReturnStep()
+    or
+    // Flow through an instance field between members of the same class
+    DataFlow::localFieldStep(pred, succ) and
+    summary = LevelStep()
+    or
+    exists(string prop |
+      basicStoreStep(pred, succ, prop) and
+      summary = StoreStep(prop)
+      or
+      basicLoadStep(pred, succ, prop) and
+      summary = LoadStep(prop)
+    )
+    or
+    any(AdditionalTypeTrackingStep st).step(pred, succ) and
+    summary = LevelStep()
+    or
+    // Store to global access path
+    exists(string name |
+      pred = AccessPath::getAnAssignmentTo(name) and
+      AccessPath::isAssignedInUniqueFile(name) and
+      succ = DataFlow::globalAccessPathRootPseudoNode() and
+      summary = StoreStep(name)
+    )
+    or
+    // Load from global access path
+    exists(string name |
+      succ = AccessPath::getAReferenceTo(name) and
+      AccessPath::isAssignedInUniqueFile(name) and
+      pred = DataFlow::globalAccessPathRootPseudoNode() and
+      summary = LoadStep(name)
+    )
+    or
+    // Store to non-global access path
+    exists(string name |
+      pred = AccessPath::getAnAssignmentTo(succ, name) and
+      summary = StoreStep(name)
+    )
+    or
+    // Load from non-global access path
+    exists(string name |
+      succ = AccessPath::getAReferenceTo(pred, name) and
+      summary = LoadStep(name) and
+      name != ""
+    )
+    or
+    // Step in/out of a promise
+    succ = PromiseTypeTracking::promiseStep(pred, summary)
+    or
+    // Summarize calls with flow directly from a parameter to a return.
+    exists(DataFlow::ParameterNode param, DataFlow::FunctionNode fun |
+      (
+        param.flowsTo(fun.getAReturn()) and
+        summary = LevelStep()
+        or
+        exists(string prop |
+          param.getAPropertyRead(prop).flowsTo(fun.getAReturn()) and
+          summary = LoadStep(prop)
+        )
+      ) and
+      if param = fun.getAParameter()
+      then
+        // Step from argument to call site.
+        argumentPassing(succ, pred, fun.getFunction(), param)
+      else (
+        // Step from captured parameter to local call sites
+        pred = param and
+        succ = fun.getAnInvocation()
+      )
+    )
+  }
+}

javascript/ql/src/semmle/javascript/frameworks/ClientRequests.qll

@@ -566,36 +566,18 @@ module ClientRequest {
    * The `isPromise` parameter reflects whether the reference is a promise containing
    * an instance of `chrome-remote-interface`, or an instance of `chrome-remote-interface`.
    */
-  private DataFlow::SourceNode chromeRemoteInterface(DataFlow::TypeTracker t, boolean isPromise) {
+  private DataFlow::SourceNode chromeRemoteInterface(DataFlow::TypeTracker t) {
-    t.start() and
     exists(DataFlow::CallNode call |
       call = DataFlow::moduleImport("chrome-remote-interface").getAnInvocation()
     |
-      result = call and isPromise = true
+      // the client is inside in a promise.
+      t.startInPromise() and result = call
       or
-      result = call.getCallback([0 .. 1]).getParameter(0) and isPromise = false
+      // the client is accessed directly using a callback.
+      t.start() and result = call.getCallback([0 .. 1]).getParameter(0)
     )
     or
-    exists(DataFlow::TypeTracker t2 | result = chromeRemoteInterface(t2, isPromise).track(t2, t))
+    exists(DataFlow::TypeTracker t2 | result = chromeRemoteInterface(t2).track(t2, t))
-    or
-    // Simple promise tracking.
-    exists(DataFlow::TypeTracker t2, DataFlow::SourceNode pred |
-      pred = chromeRemoteInterface(t2, true) and
-      isPromise = false and
-      (
-        t2 = t and
-        exists(AwaitExpr await | DataFlow::valueNode(await.getOperand()).getALocalSource() = pred |
-          result.getEnclosingExpr() = await
-        )
-        or
-        t2 = t and
-        exists(DataFlow::MethodCallNode thenCall |
-          thenCall.getMethodName() = "then" and pred = thenCall.getReceiver().getALocalSource()
-        |
-          result = thenCall.getCallback(0).getParameter(0)
-        )
-      )
-    )
   }
 
   /**
@@ -606,7 +588,7 @@ module ClientRequest {
 
     ChromeRemoteInterfaceRequest() {
       exists(DataFlow::SourceNode instance |
-        instance = chromeRemoteInterface(DataFlow::TypeTracker::end(), false)
+        instance = chromeRemoteInterface(DataFlow::TypeTracker::end())
       |
         optionsArg = 0 and
         this = instance.getAPropertyRead("Page").getAMemberCall("navigate")

javascript/ql/src/semmle/javascript/frameworks/Files.qll

@@ -75,33 +75,41 @@ private class GlobFileNameSource extends FileNameSource {
 }
 
 /**
- * A file name or an array of file names from the `globby` library.
+ * Gets a file name or an array of file names from the `globby` library.
+ * The predicate uses type-tracking. However, type-tracking is only used to track a step out of a promise.
  */
-private class GlobbyFileNameSource extends FileNameSource {
+private DataFlow::SourceNode globbyFileNameSource(DataFlow::TypeTracker t) {
-  GlobbyFileNameSource() {
   exists(string moduleName | moduleName = "globby" |
     // `require('globby').sync(_)`
-      this = DataFlow::moduleMember(moduleName, "sync").getACall()
+    t.start() and
+    result = DataFlow::moduleMember(moduleName, "sync").getACall()
     or
     // `files` in `require('globby')(_).then(files => ...)`
-      this =
+    t.startInPromise() and
-        DataFlow::moduleImport(moduleName)
+    result = DataFlow::moduleImport(moduleName).getACall()
-            .getACall()
+  )
-            .getAMethodCall("then")
+  or
-            .getCallback(0)
+  // Tracking out of a promise
-            .getParameter(0)
+  exists(DataFlow::TypeTracker t2 |
+    result = PromiseTypeTracking::promiseStep(globbyFileNameSource(t2), t, t2)
   )
-  }
 }
 
 /**
- * A file name or an array of file names from the `fast-glob` library.
+ * A file name or an array of file names from the `globby` library.
  */
-private class FastGlobFileNameSource extends FileNameSource {
+private class GlobbyFileNameSource extends FileNameSource {
-  FastGlobFileNameSource() {
+  GlobbyFileNameSource() { this = globbyFileNameSource(DataFlow::TypeTracker::end()) }
+}
+
+/**
+ * Gets a file name or an array of file names from the `fast-glob` library.
+ * The predicate uses type-tracking. However, type-tracking is only used to track a step out of a promise.
+ */
+private DataFlow::Node fastGlobFileNameSource(DataFlow::TypeTracker t) {
   exists(string moduleName | moduleName = "fast-glob" |
-      // `require('fast-glob').sync(_)`
+    // `require('fast-glob').sync(_)
-      this = DataFlow::moduleMember(moduleName, "sync").getACall()
+    t.start() and result = DataFlow::moduleMember(moduleName, "sync").getACall()
     or
     exists(DataFlow::SourceNode f |
       f = DataFlow::moduleImport(moduleName)
@@ -110,18 +118,30 @@ private class FastGlobFileNameSource extends FileNameSource {
     |
       // `files` in `require('fast-glob')(_).then(files => ...)` and
       // `files` in `require('fast-glob').async(_).then(files => ...)`
-        this = f.getACall().getAMethodCall("then").getCallback(0).getParameter(0)
+      t.startInPromise() and result = f.getACall()
     )
     or
     // `file` in `require('fast-glob').stream(_).on(_,  file => ...)`
-      this =
+    t.start() and
+    result =
       DataFlow::moduleMember(moduleName, "stream")
           .getACall()
           .getAMethodCall(EventEmitter::on())
           .getCallback(1)
           .getParameter(0)
   )
-  }
+  or
+  // Tracking out of a promise
+  exists(DataFlow::TypeTracker t2 |
+    result = PromiseTypeTracking::promiseStep(fastGlobFileNameSource(t2), t, t2)
+  )
+}
+
+/**
+ * A file name or an array of file names from the `fast-glob` library.
+ */
+private class FastGlobFileNameSource extends FileNameSource {
+  FastGlobFileNameSource() { this = fastGlobFileNameSource(DataFlow::TypeTracker::end()) }
 }
 
 /**

javascript/ql/test/library-tests/frameworks/Concepts/FileNameSource.expected

@@ -11,3 +11,7 @@
 | tst-file-names.js:25:18:25:22 | files |
 | tst-file-names.js:27:24:27:28 | files |
 | tst-file-names.js:29:27:29:30 | file |
+| tst-file-names.js:32:34:32:38 | files |
+| tst-file-names.js:34:15:34:29 | await globby(_) |
+| tst-file-names.js:36:16:36:38 | await f ... sync(_) |
+| tst-file-names.js:38:16:38:57 | await f ...  => {}) |

javascript/ql/test/library-tests/frameworks/Concepts/tst-file-names.js

@@ -27,3 +27,13 @@ fastGlob(_).then(files => files);
 fastGlob.async(_).then(files => files);
 
 fastGlob.stream(_).on(_,  file => file); // XXX
+
+async function foo() {
+  globby(_).catch(() => {}).then(files => files);
+
+  var files = await globby(_);
+
+  var files2 = await fastGlob.async(_);
+
+  var files2 = await fastGlob.async(_).catch((wat) => {});
+}

javascript/ql/test/query-tests/Security/CWE-918/RequestForgery.expected

@@ -47,6 +47,8 @@ nodes
 | tst.js:61:29:61:35 | tainted |
 | tst.js:64:30:64:36 | tainted |
 | tst.js:64:30:64:36 | tainted |
+| tst.js:68:30:68:36 | tainted |
+| tst.js:68:30:68:36 | tainted |
 edges
 | tst.js:14:9:14:52 | tainted | tst.js:18:13:18:19 | tainted |
 | tst.js:14:9:14:52 | tainted | tst.js:18:13:18:19 | tainted |
@@ -89,6 +91,8 @@ edges
 | tst.js:58:9:58:52 | tainted | tst.js:61:29:61:35 | tainted |
 | tst.js:58:9:58:52 | tainted | tst.js:64:30:64:36 | tainted |
 | tst.js:58:9:58:52 | tainted | tst.js:64:30:64:36 | tainted |
+| tst.js:58:9:58:52 | tainted | tst.js:68:30:68:36 | tainted |
+| tst.js:58:9:58:52 | tainted | tst.js:68:30:68:36 | tainted |
 | tst.js:58:19:58:42 | url.par ... , true) | tst.js:58:19:58:48 | url.par ... ).query |
 | tst.js:58:19:58:48 | url.par ... ).query | tst.js:58:19:58:52 | url.par ... ery.url |
 | tst.js:58:19:58:52 | url.par ... ery.url | tst.js:58:9:58:52 | tainted |
@@ -109,3 +113,4 @@ edges
 | tst.js:45:5:45:57 | request ... ainted) | tst.js:14:29:14:35 | req.url | tst.js:45:13:45:56 | 'http:/ ... tainted | The $@ of this request depends on $@. | tst.js:45:13:45:56 | 'http:/ ... tainted | URL | tst.js:14:29:14:35 | req.url | a user-provided value |
 | tst.js:61:2:61:37 | client. ... inted}) | tst.js:58:29:58:35 | req.url | tst.js:61:29:61:35 | tainted | The $@ of this request depends on $@. | tst.js:61:29:61:35 | tainted | URL | tst.js:58:29:58:35 | req.url | a user-provided value |
 | tst.js:64:3:64:38 | client. ... inted}) | tst.js:58:29:58:35 | req.url | tst.js:64:30:64:36 | tainted | The $@ of this request depends on $@. | tst.js:64:30:64:36 | tainted | URL | tst.js:58:29:58:35 | req.url | a user-provided value |
+| tst.js:68:3:68:38 | client. ... inted}) | tst.js:58:29:58:35 | req.url | tst.js:68:30:68:36 | tainted | The $@ of this request depends on $@. | tst.js:68:30:68:36 | tainted | URL | tst.js:58:29:58:35 | req.url | a user-provided value |

javascript/ql/test/query-tests/Security/CWE-918/tst.js

@@ -60,6 +60,10 @@ var server = http.createServer(async function(req, res) {
     var client = await CDP(options);
 	client.Page.navigate({url: tainted}); // NOT OK.
 	
+	CDP(options).catch((ignored) => {}).then((client) => {
+		client.Page.navigate({url: tainted}); // NOT OK.	
+	})
+	
 	CDP(options, (client) => {
 		client.Page.navigate({url: tainted}); // NOT OK.	
 	});