JS: Add category for promise steps

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

@@ -398,70 +398,65 @@ module PromiseFlow {
 }
 
 /**
- * Holds if taint propagates from `pred` to `succ` through promises.
+ * DEPRECATED. Use `TaintTracking::promiseStep` instead.
  */
-predicate promiseTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
-  // from `x` to `new Promise((res, rej) => res(x))`
-  pred = succ.(PromiseDefinition).getResolveParameter().getACall().getArgument(0)
-  or
-  // from `x` to `Promise.resolve(x)`
-  pred = succ.(PromiseCreationCall).getValue() and
-  not succ instanceof PromiseAllCreation
-  or
-  // from `arr` to `Promise.all(arr)`
-  pred = succ.(PromiseAllCreation).getArrayNode()
-  or
-  exists(DataFlow::MethodCallNode thn | thn.getMethodName() = "then" |
-    // from `p` to `x` in `p.then(x => ...)`
-    pred = thn.getReceiver() and
-    succ = thn.getCallback(0).getParameter(0)
-    or
-    // from `v` to `p.then(x => return v)`
-    pred = thn.getCallback([0 .. 1]).getAReturn() and
-    succ = thn
-  )
-  or
-  exists(DataFlow::MethodCallNode catch | catch.getMethodName() = "catch" |
-    // from `p` to `p.catch(..)`
-    pred = catch.getReceiver() and
-    succ = catch
-    or
-    // from `v` to `p.catch(x => return v)`
-    pred = catch.getCallback(0).getAReturn() and
-    succ = catch
-  )
-  or
-  // from `p` to `p.finally(..)`
-  exists(DataFlow::MethodCallNode finally | finally.getMethodName() = "finally" |
-    pred = finally.getReceiver() and
-    succ = finally
-  )
-  or
-  // from `x` to `await x`
-  exists(AwaitExpr await |
-    pred.getEnclosingExpr() = await.getOperand() and
-    succ.getEnclosingExpr() = await
-  )
-  or
-  exists(DataFlow::CallNode mapSeries |
-    mapSeries = DataFlow::moduleMember("bluebird", "mapSeries").getACall()
-  |
-    // from `xs` to `x` in `require("bluebird").mapSeries(xs, (x) => {...})`.
-    pred = mapSeries.getArgument(0) and
-    succ = mapSeries.getABoundCallbackParameter(1, 0)
-    or
-    // from `y` to `require("bluebird").mapSeries(x, x => y)`.
-    pred = mapSeries.getCallback(1).getAReturn() and
-    succ = mapSeries
-  )
-}
+deprecated predicate promiseTaintStep = TaintTracking::promiseStep/2;
 
-/**
- * An additional taint step that involves promises.
- */
 private class PromiseTaintStep extends TaintTracking::SharedTaintStep {
-  override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-    promiseTaintStep(pred, succ)
+  override predicate promiseStep(DataFlow::Node pred, DataFlow::Node succ) {
+    // from `x` to `new Promise((res, rej) => res(x))`
+    pred = succ.(PromiseDefinition).getResolveParameter().getACall().getArgument(0)
+    or
+    // from `x` to `Promise.resolve(x)`
+    pred = succ.(PromiseCreationCall).getValue() and
+    not succ instanceof PromiseAllCreation
+    or
+    // from `arr` to `Promise.all(arr)`
+    pred = succ.(PromiseAllCreation).getArrayNode()
+    or
+    exists(DataFlow::MethodCallNode thn | thn.getMethodName() = "then" |
+      // from `p` to `x` in `p.then(x => ...)`
+      pred = thn.getReceiver() and
+      succ = thn.getCallback(0).getParameter(0)
+      or
+      // from `v` to `p.then(x => return v)`
+      pred = thn.getCallback([0 .. 1]).getAReturn() and
+      succ = thn
+    )
+    or
+    exists(DataFlow::MethodCallNode catch | catch.getMethodName() = "catch" |
+      // from `p` to `p.catch(..)`
+      pred = catch.getReceiver() and
+      succ = catch
+      or
+      // from `v` to `p.catch(x => return v)`
+      pred = catch.getCallback(0).getAReturn() and
+      succ = catch
+    )
+    or
+    // from `p` to `p.finally(..)`
+    exists(DataFlow::MethodCallNode finally | finally.getMethodName() = "finally" |
+      pred = finally.getReceiver() and
+      succ = finally
+    )
+    or
+    // from `x` to `await x`
+    exists(AwaitExpr await |
+      pred.getEnclosingExpr() = await.getOperand() and
+      succ.getEnclosingExpr() = await
+    )
+    or
+    exists(DataFlow::CallNode mapSeries |
+      mapSeries = DataFlow::moduleMember("bluebird", "mapSeries").getACall()
+    |
+      // from `xs` to `x` in `require("bluebird").mapSeries(xs, (x) => {...})`.
+      pred = mapSeries.getArgument(0) and
+      succ = mapSeries.getABoundCallbackParameter(1, 0)
+      or
+      // from `y` to `require("bluebird").mapSeries(x, x => y)`.
+      pred = mapSeries.getCallback(1).getAReturn() and
+      succ = mapSeries
+    )
   }
 }
 

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

@@ -288,6 +288,15 @@ module TaintTracking {
      * data flow edge through data deserialization, such as `JSON.parse`.
      */
     predicate deserializeStep(DataFlow::Node pred, DataFlow::Node succ) { none() }
+
+    /**
+     * Holds if `pred` → `succ` should be considered a taint-propagating
+     * data flow edge through a promise.
+     *
+     * These steps consider a promise object to tainted if it can resolve to
+     * a tainted value.
+     */
+    predicate promiseStep(DataFlow::Node pred, DataFlow::Node succ) { none() }
   }
 
   /**
@@ -378,6 +387,18 @@ module TaintTracking {
     any(SharedTaintStep step).deserializeStep(pred, succ)
   }
 
+  /**
+   * Holds if `pred` → `succ` should be considered a taint-propagating
+   * data flow edge through data deserialization, such as `JSON.parse`.
+   *
+   * These steps consider a promise object to tainted if it can resolve to
+   * a tainted value.
+   */
+  cached
+  predicate promiseStep(DataFlow::Node pred, DataFlow::Node succ) {
+    any(SharedTaintStep step).promiseStep(pred, succ)
+  }
+
   /**
    * Holds if `pred -> succ` is a taint propagating data flow edge through a string operation.
    */
@@ -409,7 +430,8 @@ module TaintTracking {
     stringConcatenationStep(pred, succ) or
     stringManipulationStep(pred, succ) or
     serializeStep(pred, succ) or
-    deserializeStep(pred, succ)
+    deserializeStep(pred, succ) or
+    promiseStep(pred, succ)
   }
 
   /**

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

@@ -275,7 +275,7 @@ module Firebase {
       result.hasUnderlyingType("firebase", "database.DataSnapshot")
     )
     or
-    promiseTaintStep(snapshot(t), result)
+    TaintTracking::promiseStep(snapshot(t), result)
     or
     exists(DataFlow::TypeTracker t2 | result = snapshot(t2).track(t2, t))
   }

javascript/ql/src/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll

@@ -659,7 +659,7 @@ module TaintedPath {
       )
     )
     or
-    promiseTaintStep(src, dst) and srclabel = dstlabel
+    TaintTracking::promiseStep(src, dst) and srclabel = dstlabel
     or
     TaintTracking::persistentStorageStep(src, dst) and srclabel = dstlabel
     or