JavaScript: Switch AdditionalTaintStep to extend a unit type.

This is a backwards-incompatible change, since any existing subtypes of `AdditionalTaintStep`
become invalid.

javascript/change-notes/2021-02-18-redux.md

@@ -0,0 +1,3 @@
+lgtm,codescanning
+* Support for Redux has improved. The security queries can now track taint through reducer functions and state managed by Redux.
+  Affected packages are `redux`, `react-redux`, `@reduxjs/toolkit`, `redux-actions`, `redux-persist`, `reduce-reducers`, `redux-immutable`, and `immer`.

javascript/ql/src/javascript.qll

@@ -100,6 +100,7 @@ import semmle.javascript.frameworks.PkgCloud
 import semmle.javascript.frameworks.PropertyProjection
 import semmle.javascript.frameworks.React
 import semmle.javascript.frameworks.ReactNative
+import semmle.javascript.frameworks.Redux
 import semmle.javascript.frameworks.Request
 import semmle.javascript.frameworks.RxJS
 import semmle.javascript.frameworks.ServerLess

javascript/ql/src/semmle/javascript/Arrays.qll

@@ -9,12 +9,9 @@ module ArrayTaintTracking {
   /**
    * A taint propagating data flow edge caused by the builtin array functions.
    */
-  private class ArrayFunctionTaintStep extends TaintTracking::AdditionalTaintStep,
-    DataFlow::CallNode {
-    ArrayFunctionTaintStep() { arrayFunctionTaintStep(_, _, this) }
-
+  private class ArrayFunctionTaintStep extends TaintTracking::AdditionalTaintStep{
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      arrayFunctionTaintStep(pred, succ, this)
+      arrayFunctionTaintStep(pred, succ, _)
     }
   }
 

javascript/ql/src/semmle/javascript/Base64.qll

@@ -68,13 +68,11 @@ module Base64 {
    * to avoid false positives.
    */
   private class Base64DecodingStep extends TaintTracking::AdditionalTaintStep {
-    Decode dec;
-
-    Base64DecodingStep() { this = dec }
-
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = dec.getInput() and
-      succ = dec.getOutput()
+      exists(Decode dec |
+        pred = dec.getInput() and
+        succ = dec.getOutput()
+      )
     }
   }
 }

javascript/ql/src/semmle/javascript/BasicBlocks.qll

@@ -303,7 +303,7 @@ class ReachableBasicBlock extends BasicBlock {
   /**
    * Holds if this basic block strictly dominates `bb`.
    */
-  cached
+  pragma[inline]
   predicate strictlyDominates(ReachableBasicBlock bb) { bbIDominates+(this, bb) }
 
   /**
@@ -311,15 +311,13 @@ class ReachableBasicBlock extends BasicBlock {
    *
    * This predicate is reflexive: each reachable basic block dominates itself.
    */
-  predicate dominates(ReachableBasicBlock bb) {
-    bb = this or
-    strictlyDominates(bb)
-  }
+  pragma[inline]
+  predicate dominates(ReachableBasicBlock bb) { bbIDominates*(this, bb) }
 
   /**
    * Holds if this basic block strictly post-dominates `bb`.
    */
-  cached
+  pragma[inline]
   predicate strictlyPostDominates(ReachableBasicBlock bb) { bbIPostDominates+(this, bb) }
 
   /**
@@ -327,10 +325,8 @@ class ReachableBasicBlock extends BasicBlock {
    *
    * This predicate is reflexive: each reachable basic block post-dominates itself.
    */
-  predicate postDominates(ReachableBasicBlock bb) {
-    bb = this or
-    strictlyPostDominates(bb)
-  }
+  pragma[inline]
+  predicate postDominates(ReachableBasicBlock bb) { bbIPostDominates*(this, bb) }
 }
 
 /**

javascript/ql/src/semmle/javascript/Extend.qll

@@ -166,13 +166,11 @@ private class FunctionalExtendCallShallow extends ExtendCall {
  * and to the source of the destination object.
  */
 private class ExtendCallTaintStep extends TaintTracking::AdditionalTaintStep {
-  ExtendCall extend;
-
-  ExtendCallTaintStep() { this = extend }
-
   override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-    pred = extend.getASourceOperand() and succ = extend.getDestinationOperand().getALocalSource()
-    or
-    pred = extend.getAnOperand() and succ = extend
+    exists(ExtendCall extend |
+      pred = extend.getASourceOperand() and succ = extend.getDestinationOperand().getALocalSource()
+      or
+      pred = extend.getAnOperand() and succ = extend
+    )
   }
 }

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

@@ -460,13 +460,7 @@ predicate promiseTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
  * An additional taint step that involves promises.
  */
 private class PromiseTaintStep extends TaintTracking::AdditionalTaintStep {
-  DataFlow::Node source;
-
-  PromiseTaintStep() { promiseTaintStep(source, this) }
-
-  override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-    pred = source and succ = this
-  }
+  override predicate step(DataFlow::Node pred, DataFlow::Node succ) { promiseTaintStep(pred, succ) }
 }
 
 /**
@@ -500,14 +494,13 @@ private module AsyncReturnSteps {
   /**
    * A data-flow step for ordinary return from an async function in a taint configuration.
    */
-  private class AsyncTaintReturn extends TaintTracking::AdditionalTaintStep, DataFlow::FunctionNode {
-    Function f;
-
-    AsyncTaintReturn() { this.getFunction() = f and f.isAsync() }
-
+  private class AsyncTaintReturn extends TaintTracking::AdditionalTaintStep {
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      returnExpr(f, pred, _) and
-      succ.(DataFlow::FunctionReturnNode).getFunction() = f
+      exists(Function f |
+        f.isAsync() and
+        returnExpr(f, pred, _) and
+        succ.(DataFlow::FunctionReturnNode).getFunction() = f
+      )
     }
   }
 }
@@ -617,13 +610,11 @@ private module ClosurePromise {
    * Taint steps through closure promise methods.
    */
   private class ClosurePromiseTaintStep extends TaintTracking::AdditionalTaintStep {
-    DataFlow::Node pred;
-
-    ClosurePromiseTaintStep() {
+    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
       // static methods in goog.Promise
       exists(DataFlow::CallNode call, string name |
         call = Closure::moduleImport("goog.Promise." + name).getACall() and
-        this = call and
+        succ = call and
         pred = call.getAnArgument()
       |
         name = "all" or
@@ -635,12 +626,10 @@ private module ClosurePromise {
       // promise created through goog.promise.withResolver()
       exists(DataFlow::CallNode resolver |
         resolver = Closure::moduleImport("goog.Promise.withResolver").getACall() and
-        this = resolver.getAPropertyRead("promise") and
+        succ = resolver.getAPropertyRead("promise") and
         pred = resolver.getAMethodCall("resolve").getArgument(0)
       )
     }
-
-    override predicate step(DataFlow::Node src, DataFlow::Node dst) { src = pred and dst = this }
   }
 }
 

javascript/ql/src/semmle/javascript/RangeAnalysis.qll

@@ -606,10 +606,10 @@ module RangeAnalysis {
         cfg2BB = cfg2.getBasicBlock() and
         cfg2RBB = cfg2BB.(ReachableBasicBlock) and
         (
-          cfg1RBB.strictlyDominates(cfg2BB) and
+          cfg2BB.getImmediateDominator+() = cfg1RBB and
           cfg = cfg2
           or
-          cfg2RBB.strictlyDominates(cfg1RBB) and
+          cfg1BB.getImmediateDominator+() = cfg2BB and
           cfg = cfg1
         )
       )
@@ -681,7 +681,7 @@ module RangeAnalysis {
       midBB = midcfg.getBasicBlock() and
       midRBB = midBB.(ReachableBasicBlock) and
       cfgBB = cfg.getBasicBlock() and
-      midRBB.strictlyDominates(cfgBB)
+      cfgBB.getImmediateDominator+() = midRBB
     )
   }
 

javascript/ql/src/semmle/javascript/Unit.qll

@@ -0,0 +1,10 @@
+/** Provides the `Unit` class. */
+
+/** The unit type. */
+private newtype TUnit = TMkUnit()
+
+/** The trivial type with a single element. */
+class Unit extends TUnit {
+  /** Gets a textual representation of this element. */
+  string toString() { result = "unit" }
+}

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

@@ -1660,4 +1660,59 @@ module DataFlow {
   import TypeTracking
 
   predicate localTaintStep = TaintTracking::localTaintStep/2;
+
+  /**
+   * Holds if the function in `succ` forwards all its arguments to a call to `pred` and returns
+   * its result. This can thus be seen as a step `pred -> succ` used for tracking function values
+   * through "wrapper functions", since the `succ` function partially replicates behavior of `pred`.
+   *
+   * Examples:
+   * ```js
+   * function f(x) {
+   *   return g(x); // step: g -> f
+   * }
+   *
+   * function doExec(x) {
+   *   console.log(x);
+   *   return exec(x); // step: exec -> doExec
+   * }
+   *
+   * function doEither(x, y) {
+   *   if (x > y) {
+   *     return foo(x, y); // step: foo -> doEither
+   *   } else {
+   *     return bar(x, y); // step: bar -> doEither
+   *   }
+   * }
+   *
+   * function wrapWithLogging(f) {
+   *   return (x) => {
+   *     console.log(x);
+   *     return f(x); // step: f -> anonymous function
+   *   }
+   * }
+   * wrapWithLogging(g); // step: g -> wrapWithLogging(g)
+   * ```
+   */
+  predicate functionForwardingStep(DataFlow::Node pred, DataFlow::Node succ) {
+    exists(DataFlow::FunctionNode function, DataFlow::CallNode call |
+      call.flowsTo(function.getReturnNode()) and
+      forall(int i | exists([call.getArgument(i), function.getParameter(i)]) |
+        function.getParameter(i).flowsTo(call.getArgument(i))
+      ) and
+      pred = call.getCalleeNode() and
+      succ = function
+    )
+    or
+    // Given a generic wrapper function like,
+    //
+    //   function wrap(f) { return (x, y) => f(x, y) };
+    //
+    // add steps through calls to that function: `g -> wrap(g)`
+    exists(DataFlow::FunctionNode wrapperFunction, SourceNode param, Node paramUse |
+      FlowSteps::argumentPassing(succ, pred, wrapperFunction.getFunction(), param) and
+      param.flowsTo(paramUse) and
+      functionForwardingStep(paramUse, wrapperFunction.getReturnNode().getALocalSource())
+    )
+  }
 }

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

@@ -47,6 +47,11 @@ class SourceNode extends DataFlow::Node {
    */
   predicate flowsToExpr(Expr sink) { flowsTo(DataFlow::valueNode(sink)) }
 
+  /**
+   * Gets a node into which data may flow from this node in zero or more local steps.
+   */
+  DataFlow::Node getALocalUse() { flowsTo(result) }
+
   /**
    * Gets a reference (read or write) of property `propName` on this node.
    */

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

@@ -206,6 +206,8 @@ module TaintTracking {
     override predicate sanitizes(boolean outcome, Expr e) { none() }
   }
 
+  private newtype TUnit = TUnitInjector()
+
   /**
    * A taint-propagating data flow edge that should be added to all taint tracking
    * configurations in addition to standard data flow edges.
@@ -214,14 +216,15 @@ module TaintTracking {
    * of the standard library. Override `Configuration::isAdditionalTaintStep`
    * for analysis-specific taint steps.
    */
-  cached
-  abstract class AdditionalTaintStep extends DataFlow::Node {
+  class AdditionalTaintStep extends TUnit {
     /**
      * Holds if `pred` → `succ` should be considered a taint-propagating
      * data flow edge.
      */
     cached
     abstract predicate step(DataFlow::Node pred, DataFlow::Node succ);
+
+    string toString() { result = "Additional taint step class" }
   }
 
   /**
@@ -229,11 +232,7 @@ module TaintTracking {
    * promises.
    */
   private class HeapTaintStep extends AdditionalTaintStep {
-    HeapTaintStep() { heapStep(_, this) }
-
-    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      heapStep(pred, succ) and succ = this
-    }
+    override predicate step(DataFlow::Node pred, DataFlow::Node succ) { heapStep(pred, succ) }
   }
 
   /**
@@ -280,16 +279,15 @@ module TaintTracking {
    * A taint propagating data flow edge through persistent storage.
    */
   class PersistentStorageTaintStep extends AdditionalTaintStep {
-    PersistentReadAccess read;
-
-    PersistentStorageTaintStep() { this = read }
-
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = read.getAWrite().getValue() and
-      succ = read
+      persistentStorageTaintStep(pred, succ)
     }
   }
 
+  predicate persistentStorageTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
+    pred = succ.(PersistentReadAccess).getAWrite().getValue()
+  }
+
   predicate arrayFunctionTaintStep = ArrayTaintTracking::arrayFunctionTaintStep/3;
 
   /**
@@ -303,10 +301,7 @@ module TaintTracking {
    * map to be tainted as soon as one of its entries is.
    */
   private class DictionaryTaintStep extends AdditionalTaintStep {
-    DictionaryTaintStep() { dictionaryTaintStep(_, this) }
-
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      succ = this and
       dictionaryTaintStep(pred, succ)
     }
   }
@@ -328,9 +323,7 @@ module TaintTracking {
    * also is an instance of `C`.
    */
   private class ReactComponentStateTaintStep extends AdditionalTaintStep {
-    DataFlow::Node source;
-
-    ReactComponentStateTaintStep() {
+    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
       exists(ReactComponent c, DataFlow::PropRead prn, DataFlow::PropWrite pwn |
         (
           c.getACandidateStateSource().flowsTo(pwn.getBase()) or
@@ -342,14 +335,10 @@ module TaintTracking {
         )
       |
         prn.getPropertyName() = pwn.getPropertyName() and
-        this = prn and
-        source = pwn.getRhs()
+        succ = prn and
+        pred = pwn.getRhs()
       )
     }
-
-    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = source and succ = this
-    }
   }
 
   /**
@@ -359,21 +348,15 @@ module TaintTracking {
    * also is an instance of `C`.
    */
   private class ReactComponentPropsTaintStep extends AdditionalTaintStep {
-    DataFlow::Node source;
-
-    ReactComponentPropsTaintStep() {
+    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
       exists(ReactComponent c, string name, DataFlow::PropRead prn |
         prn = c.getAPropRead(name) or
         prn = c.getAPreviousPropsSource().getAPropertyRead(name)
       |
-        source = c.getACandidatePropsValue(name) and
-        this = prn
+        pred = c.getACandidatePropsValue(name) and
+        succ = prn
       )
     }
-
-    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = source and succ = this
-    }
   }
 
   /**
@@ -383,10 +366,7 @@ module TaintTracking {
    * we consider any `+` operation to propagate taint.
    */
   class StringConcatenationTaintStep extends AdditionalTaintStep {
-    StringConcatenationTaintStep() { StringConcatenation::taintStep(_, this) }
-
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      succ = this and
       StringConcatenation::taintStep(pred, succ)
     }
   }
@@ -395,11 +375,8 @@ module TaintTracking {
    * A taint propagating data flow edge arising from string manipulation
    * functions defined in the standard library.
    */
-  private class StringManipulationTaintStep extends AdditionalTaintStep, DataFlow::ValueNode {
-    StringManipulationTaintStep() { stringManipulationStep(_, this) }
-
+  private class StringManipulationTaintStep extends AdditionalTaintStep {
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      succ = this and
       stringManipulationStep(pred, succ)
     }
   }
@@ -488,16 +465,8 @@ module TaintTracking {
    * A taint propagating data flow edge arising from string formatting.
    */
   private class StringFormattingTaintStep extends AdditionalTaintStep {
-    PrintfStyleCall call;
-
-    StringFormattingTaintStep() {
-      this = call and
-      call.returnsFormatted()
-    }
-
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      succ = this and
-      (
+      exists(PrintfStyleCall call | call.returnsFormatted() and succ = call |
         pred = call.getFormatString()
         or
         pred = call.getFormatArgument(_)
@@ -510,59 +479,50 @@ module TaintTracking {
    * `RegExp.prototype.exec` to its result.
    */
   private class RegExpExecTaintStep extends AdditionalTaintStep {
-    DataFlow::MethodCallNode self;
-
-    RegExpExecTaintStep() {
-      this = self and
-      self.getReceiver().analyze().getAType() = TTRegExp() and
-      self.getMethodName() = "exec" and
-      self.getNumArgument() = 1
-    }
-
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = self.getArgument(0) and
-      succ = this
+      exists(DataFlow::MethodCallNode call |
+        call.getReceiver().analyze().getAType() = TTRegExp() and
+        call.getMethodName() = "exec" and
+        call.getNumArgument() = 1 and
+        pred = call.getArgument(0) and
+        succ = call
+      )
     }
   }
 
   /**
    * A taint propagating data flow edge arising from calling `String.prototype.match()`.
    */
-  private class StringMatchTaintStep extends AdditionalTaintStep, DataFlow::MethodCallNode {
-    StringMatchTaintStep() {
-      this.getMethodName() = "match" and
-      this.getNumArgument() = 1 and
-      this.getArgument(0).analyze().getAType() = TTRegExp()
-    }
-
+  private class StringMatchTaintStep extends AdditionalTaintStep {
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = this.getReceiver() and
-      succ = this
+      exists(DataFlow::MethodCallNode call |
+        pred = call.getReceiver() and
+        succ = call and
+        call.getMethodName() = "match" and
+        call.getNumArgument() = 1 and
+        call.getArgument(0).analyze().getAType() = TTRegExp()
+      )
     }
   }
 
   /**
    * A taint propagating data flow edge arising from JSON unparsing.
    */
-  private class JsonStringifyTaintStep extends AdditionalTaintStep, DataFlow::CallNode {
-    JsonStringifyTaintStep() { this instanceof JsonStringifyCall }
-
+  private class JsonStringifyTaintStep extends AdditionalTaintStep {
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = getArgument(0) and succ = this
+      pred = succ.(JsonStringifyCall).getArgument(0)
     }
   }
 
   /**
    * A taint propagating data flow edge arising from JSON parsing.
    */
-  private class JsonParserTaintStep extends AdditionalTaintStep, DataFlow::CallNode {
-    JsonParserCall call;
-
-    JsonParserTaintStep() { this = call }
-
+  private class JsonParserTaintStep extends AdditionalTaintStep {
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = call.getInput() and
-      succ = call.getOutput()
+      exists(JsonParserCall call |
+        pred = call.getInput() and
+        succ = call.getOutput()
+      )
     }
   }
 
@@ -664,33 +624,29 @@ module TaintTracking {
   /**
    * A taint propagating data flow edge arising from sorting.
    */
-  private class SortTaintStep extends AdditionalTaintStep, DataFlow::MethodCallNode {
-    SortTaintStep() { getMethodName() = "sort" }
-
+  private class SortTaintStep extends AdditionalTaintStep {
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = getReceiver() and succ = this
+      exists(DataFlow::MethodCallNode call | call.getMethodName() = "sort" |
+        pred = call.getReceiver() and succ = call
+      )
     }
   }
 
   /**
    * A taint step through an exception constructor, such as `x` to `new Error(x)`.
    */
-  class ErrorConstructorTaintStep extends AdditionalTaintStep, DataFlow::InvokeNode {
-    ErrorConstructorTaintStep() {
-      exists(string name | this = DataFlow::globalVarRef(name).getAnInvocation() |
-        name = "Error" or
-        name = "EvalError" or
-        name = "RangeError" or
-        name = "ReferenceError" or
-        name = "SyntaxError" or
-        name = "TypeError" or
-        name = "URIError"
-      )
-    }
-
+  class ErrorConstructorTaintStep extends AdditionalTaintStep {
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = getArgument(0) and
-      succ = this
+      exists(DataFlow::InvokeNode call |
+        call =
+          DataFlow::globalVarRef([
+              "Error", "EvalError", "RangeError", "ReferenceError", "SyntaxError", "TypeError",
+              "URIError"
+            ]).getAnInvocation()
+      |
+        pred = call.getArgument(0) and
+        succ = call
+      )
     }
   }
 
@@ -756,11 +712,8 @@ module TaintTracking {
     }
 
     private class StaticRegExpCaptureStep extends AdditionalTaintStep {
-      StaticRegExpCaptureStep() { staticRegExpCaptureStep(this, _) }
-
       override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-        pred = this and
-        staticRegExpCaptureStep(this, succ)
+        staticRegExpCaptureStep(pred, succ)
       }
     }
   }

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

@@ -4,12 +4,7 @@
  */
 
 private import javascript
-
-private newtype TUnit = MkUnit()
-
-private class Unit extends TUnit {
-  string toString() { result = "unit" }
-}
+private import semmle.javascript.Unit
 
 /**
  * Internal extension point for adding flow edges prior to call graph construction

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

@@ -166,8 +166,6 @@ module Angular2 {
   }
 
   private class AngularTaintStep extends TaintTracking::AdditionalTaintStep {
-    AngularTaintStep() { taintStep(_, this) }
-
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) { taintStep(pred, succ) }
   }
 
@@ -469,13 +467,11 @@ module Angular2 {
    * a step from `array` to each access to `elem`.
    */
   private class ForLoopStep extends TaintTracking::AdditionalTaintStep {
-    ForLoopAttribute attrib;
-
-    ForLoopStep() { this = attrib.getIterationDomain() }
-
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = this and
-      succ = attrib.getAnIteratorAccess()
+      exists(ForLoopAttribute attrib |
+        pred = attrib.getIterationDomain() and
+        succ = attrib.getAnIteratorAccess()
+      )
     }
   }
 
@@ -498,27 +494,23 @@ module Angular2 {
     result.getCalleeNode().asExpr().(PipeRefExpr).getName() = name
   }
 
-  private class BuiltinPipeStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
-    string name;
-
-    BuiltinPipeStep() { this = getAPipeCall(name) }
-
+  private class BuiltinPipeStep extends TaintTracking::AdditionalTaintStep {
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      succ = this and
-      exists(int i | pred = getArgument(i) |
-        i = 0 and
-        name =
-          [
-            "async", "i18nPlural", "json", "keyvalue", "lowercase", "uppercase", "titlecase",
-            "slice"
-          ]
+      exists(DataFlow::CallNode call, string name | call = getAPipeCall(name) and succ = call |
+        exists(int i | pred = call.getArgument(i) |
+          i = 0 and
+          name =
+            [
+              "async", "i18nPlural", "json", "keyvalue", "lowercase", "uppercase", "titlecase",
+              "slice"
+            ]
+          or
+          i = 1 and name = "date" // date format string
+        )
         or
-        i = 1 and name = "date" // date format string
+        name = "translate" and
+        pred = [call.getArgument(1), call.getOptionArgument(1, _)]
       )
-      or
-      name = "translate" and
-      succ = this and
-      pred = [getArgument(1), getOptionArgument(1, _)]
     }
   }
 
@@ -568,26 +560,25 @@ module Angular2 {
    * ```
    */
   private class MatTableTaintStep extends TaintTracking::AdditionalTaintStep {
-    MatTableElement table;
-
-    MatTableTaintStep() { this = table.getDataSourceNode() }
-
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = this and
-      succ = table.getARowRef()
+      exists(MatTableElement table |
+        pred = table.getDataSourceNode() and
+        succ = table.getARowRef()
+      )
     }
   }
 
   /** A taint step into the data array of a `MatTableDataSource` instance. */
-  private class MatTableDataSourceStep extends TaintTracking::AdditionalTaintStep, DataFlow::NewNode {
-    MatTableDataSourceStep() {
-      this =
-        DataFlow::moduleMember("@angular/material/table", "MatTableDataSource").getAnInstantiation()
-    }
-
+  private class MatTableDataSourceStep extends TaintTracking::AdditionalTaintStep {
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = [getArgument(0), getAPropertyWrite("data").getRhs()] and
-      succ = this
+      exists(DataFlow::NewNode new |
+        new =
+          DataFlow::moduleMember("@angular/material/table", "MatTableDataSource")
+              .getAnInstantiation()
+      |
+        pred = [new.getArgument(0), new.getAPropertyWrite("data").getRhs()] and
+        succ = new
+      )
     }
   }
 }

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

@@ -150,11 +150,11 @@ module AsyncPackage {
    *
    * For example: `data -> item` in `async.each(data, (item, cb) => {})`.
    */
-  private class IterationInputTaintStep extends TaintTracking::AdditionalTaintStep, IterationCall {
+  private class IterationInputTaintStep extends TaintTracking::AdditionalTaintStep {
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      exists(DataFlow::FunctionNode iteratee |
-        iteratee = getIteratorCallback() and // Require a closure to avoid spurious call/return mismatch.
-        pred = getCollection() and
+      exists(IterationCall iteration, DataFlow::FunctionNode iteratee |
+        iteratee = iteration.getIteratorCallback() and // Require a closure to avoid spurious call/return mismatch.
+        pred = iteration.getCollection() and
         succ = iteratee.getParameter(0)
       )
     }
@@ -166,18 +166,15 @@ module AsyncPackage {
    *
    * For example: `item + taint()` -> result` in `async.map(data, (item, cb) => cb(null, item + taint()), (err, result) => {})`.
    */
-  private class IterationOutputTaintStep extends TaintTracking::AdditionalTaintStep, IterationCall {
-    IterationOutputTaintStep() {
-      name = "concat" or
-      name = "map" or
-      name = "reduce" or
-      name = "reduceRight"
-    }
-
+  private class IterationOutputTaintStep extends TaintTracking::AdditionalTaintStep {
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      exists(DataFlow::FunctionNode iteratee, DataFlow::FunctionNode final, int i |
-        iteratee = getIteratorCallback().getALocalSource() and
-        final = getFinalCallback() and // Require a closure to avoid spurious call/return mismatch.
+      exists(
+        IterationCall iteration, DataFlow::FunctionNode iteratee, DataFlow::FunctionNode final,
+        int i
+      |
+        iteration.getName() = ["concat", "map", "reduce", "reduceRight"] and
+        iteratee = iteration.getIteratorCallback().getALocalSource() and
+        final = iteration.getFinalCallback() and // Require a closure to avoid spurious call/return mismatch.
         pred = getLastParameter(iteratee).getACall().getArgument(i) and
         succ = final.getParameter(i)
       )
@@ -189,16 +186,13 @@ module AsyncPackage {
    *
    * For example: `data -> result` in `async.sortBy(data, orderingFn, (err, result) => {})`.
    */
-  private class IterationPreserveTaintStep extends TaintTracking::AdditionalTaintStep, IterationCall {
-    IterationPreserveTaintStep() {
-      name = "sortBy"
-      // We don't currently include `filter` and `reject` as they could act as sanitizers.
-    }
-
+  private class IterationPreserveTaintStep extends TaintTracking::AdditionalTaintStep {
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      exists(DataFlow::FunctionNode final |
-        final = getFinalCallback() and // Require a closure to avoid spurious call/return mismatch.
-        pred = getCollection() and
+      exists(IterationCall iteration, DataFlow::FunctionNode final |
+        // We don't currently include `filter` and `reject` as they could act as sanitizers.
+        iteration.getName() = "sortBy" and
+        final = iteration.getFinalCallback() and // Require a closure to avoid spurious call/return mismatch.
+        pred = iteration.getCollection() and
         succ = final.getParameter(1)
       )
     }

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

@@ -8,32 +8,25 @@ private DataFlow::SourceNode classnames() {
   result = DataFlow::moduleImport(["classnames", "classnames/dedupe", "classnames/bind"])
 }
 
-private class PlainStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
-  PlainStep() {
-    this = classnames().getACall()
-    or
-    this = DataFlow::moduleImport("clsx").getACall()
-  }
-
+private class PlainStep extends TaintTracking::AdditionalTaintStep {
   override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-    pred = getAnArgument() and
-    succ = this
+    exists(DataFlow::CallNode call |
+      call = classnames().getACall() or call = DataFlow::moduleImport("clsx").getACall()
+    |
+      pred = call.getAnArgument() and
+      succ = call
+    )
   }
 }
 
 /**
  * Step from `x` or `y` to the result of `classnames.bind(x)(y)`.
  */
-private class BindStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
-  DataFlow::CallNode bind;
-
-  BindStep() {
-    bind = classnames().getAMemberCall("bind") and
-    this = bind.getACall()
-  }
-
+private class BindStep extends TaintTracking::AdditionalTaintStep {
   override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-    pred = [getAnArgument(), bind.getAnArgument(), bind.getOptionArgument(_, _)] and
-    succ = this
+    exists(DataFlow::CallNode bind | bind = classnames().getAMemberCall("bind") |
+      pred = [succ.(DataFlow::CallNode).getAnArgument(), bind.getAnArgument(), bind.getOptionArgument(_, _)] and
+      succ = bind.getACall()
+    )
   }
 }

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

@@ -7,12 +7,13 @@ import javascript
 module ClosureLibrary {
   private import DataFlow
 
-  private class StringStep extends TaintTracking::AdditionalTaintStep, CallNode {
-    Node pred;
-
-    StringStep() {
-      exists(string name | this = Closure::moduleImport("goog.string." + name).getACall() |
-        pred = getAnArgument() and
+  private class StringStep extends TaintTracking::AdditionalTaintStep {
+    override predicate step(Node pred, Node succ) {
+      exists(CallNode call, string name |
+        call = Closure::moduleImport("goog.string." + name).getACall() and
+        succ = call
+      |
+        pred = call.getAnArgument() and
         (
           name = "canonicalizeNewlines" or
           name = "capitalize" or
@@ -39,7 +40,7 @@ module ClosureLibrary {
           name = "whitespaceEscape"
         )
         or
-        pred = getArgument(0) and
+        pred = call.getArgument(0) and
         (
           name = "truncate" or
           name = "truncateMiddle" or
@@ -47,10 +48,5 @@ module ClosureLibrary {
         )
       )
     }
-
-    override predicate step(Node src, Node dst) {
-      src = pred and
-      dst = this
-    }
   }
 }

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

@@ -88,7 +88,7 @@ module FunctionCompositionCall {
     RightToLeft() {
       this = DataFlow::moduleImport(["compose-function"]).getACall()
       or
-      this = DataFlow::moduleMember(["redux", "ramda"], "compose").getACall()
+      this = DataFlow::moduleMember(["redux", "ramda", "@reduxjs/toolkit"], "compose").getACall()
       or
       this = LodashUnderscore::member("flowRight").getACall()
     }
@@ -114,33 +114,27 @@ module FunctionCompositionCall {
  * A taint step for a composed function.
  */
 private class ComposedFunctionTaintStep extends TaintTracking::AdditionalTaintStep {
-  FunctionCompositionCall composed;
-  DataFlow::CallNode call;
-
-  ComposedFunctionTaintStep() {
-    call = composed.getACall() and
-    this = call
-  }
-
   override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-    exists(int fnIndex, DataFlow::FunctionNode fn | fn = composed.getOperandFunction(fnIndex) |
-      // flow into the first function
-      fnIndex = composed.getNumOperand() - 1 and
-      exists(int callArgIndex |
-        pred = call.getArgument(callArgIndex) and
-        succ = fn.getParameter(callArgIndex)
+    exists(FunctionCompositionCall composed, DataFlow::CallNode call | call = composed.getACall() |
+      exists(int fnIndex, DataFlow::FunctionNode fn | fn = composed.getOperandFunction(fnIndex) |
+        // flow into the first function
+        fnIndex = composed.getNumOperand() - 1 and
+        exists(int callArgIndex |
+          pred = call.getArgument(callArgIndex) and
+          succ = fn.getParameter(callArgIndex)
+        )
+        or
+        // flow through the composed functions
+        exists(DataFlow::FunctionNode predFn | predFn = composed.getOperandFunction(fnIndex + 1) |
+          pred = predFn.getReturnNode() and
+          succ = fn.getParameter(0)
+        )
+        or
+        // flow out of the composed call
+        fnIndex = 0 and
+        pred = fn.getReturnNode() and
+        succ = call
       )
-      or
-      // flow through the composed functions
-      exists(DataFlow::FunctionNode predFn | predFn = composed.getOperandFunction(fnIndex + 1) |
-        pred = predFn.getReturnNode() and
-        succ = fn.getParameter(0)
-      )
-      or
-      // flow out of the composed call
-      fnIndex = 0 and
-      pred = fn.getReturnNode() and
-      succ = this
     )
   }
 }

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

@@ -29,24 +29,24 @@ private module DateFns {
    *
    * A format string can use single-quotes to include mostly arbitrary text.
    */
-  private class FormatStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
-    FormatStep() { this = formatFunction().getACall() }
-
+  private class FormatStep extends TaintTracking::AdditionalTaintStep {
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = getArgument(1) and
-      succ = this
+      exists(DataFlow::CallNode call | call = formatFunction().getACall() |
+        pred = call.getArgument(1) and
+        succ = call
+      )
     }
   }
 
   /**
    * Taint step of form: `f -> format(f)(date)`
    */
-  private class CurriedFormatStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
-    CurriedFormatStep() { this = curriedFormatFunction().getACall() }
-
+  private class CurriedFormatStep extends TaintTracking::AdditionalTaintStep {
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = getArgument(0) and
-      succ = getACall()
+      exists(DataFlow::CallNode call | call = curriedFormatFunction().getACall() |
+        pred = call.getArgument(0) and
+        succ = call.getACall()
+      )
     }
   }
 }
@@ -66,12 +66,12 @@ private module Moment {
    *
    * The format string can use backslash-escaping to include mostly arbitrary text.
    */
-  private class MomentFormatStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
-    MomentFormatStep() { this = moment().getMember("format").getACall() }
-
+  private class MomentFormatStep extends TaintTracking::AdditionalTaintStep {
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = getArgument(0) and
-      succ = this
+      exists(DataFlow::CallNode call | call = moment().getMember("format").getACall() |
+        pred = call.getArgument(0) and
+        succ = call
+      )
     }
   }
 }
@@ -82,12 +82,12 @@ private module DateFormat {
    *
    * The format string can use single-quotes to include mostly arbitrary text.
    */
-  private class DateFormatStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
-    DateFormatStep() { this = DataFlow::moduleImport("dateformat").getACall() }
-
+  private class DateFormatStep extends TaintTracking::AdditionalTaintStep {
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = getArgument(1) and
-      succ = this
+      exists(DataFlow::CallNode call | call = DataFlow::moduleImport("dateformat").getACall() |
+        pred = call.getArgument(1) and
+        succ = call
+      )
     }
   }
 }

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

@@ -11,12 +11,12 @@ private module JwtDecode {
   /**
    * A taint-step for `succ = require("jwt-decode")(pred)`.
    */
-  private class JwtDecodeStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
-    JwtDecodeStep() { this = DataFlow::moduleImport("jwt-decode").getACall() }
-
+  private class JwtDecodeStep extends TaintTracking::AdditionalTaintStep {
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = this.getArgument(0) and
-      succ = this
+      exists(DataFlow::CallNode call | call = DataFlow::moduleImport("jwt-decode").getACall() |
+        pred = call.getArgument(0) and
+        succ = call
+      )
     }
   }
 }
@@ -28,12 +28,14 @@ private module JsonWebToken {
   /**
    * A taint-step for `require("jsonwebtoken").verify(pred, "key", (err succ) => {...})`.
    */
-  private class VerifyStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
-    VerifyStep() { this = DataFlow::moduleMember("jsonwebtoken", "verify").getACall() }
-
+  private class VerifyStep extends TaintTracking::AdditionalTaintStep {
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = this.getArgument(0) and
-      succ = this.getABoundCallbackParameter(2, 1)
+      exists(DataFlow::CallNode call |
+        call = DataFlow::moduleMember("jsonwebtoken", "verify").getACall()
+      |
+        pred = call.getArgument(0) and
+        succ = call.getABoundCallbackParameter(2, 1)
+      )
     }
   }
 

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

@@ -441,10 +441,8 @@ module LodashUnderscore {
    * A model for taint-steps involving (non-function) underscore methods.
    */
   private class UnderscoreTaintStep extends TaintTracking::AdditionalTaintStep {
-    UnderscoreTaintStep() { underscoreTaintStep(this, _) }
-
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      underscoreTaintStep(pred, succ) and pred = this
+      underscoreTaintStep(pred, succ)
     }
   }
 }

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

@@ -7,45 +7,45 @@ import javascript
 /**
  * A taint step for the `marked` library, that converts markdown to HTML.
  */
-private class MarkedStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
-  MarkedStep() {
-    this = DataFlow::globalVarRef("marked").getACall()
-    or
-    this = DataFlow::moduleImport("marked").getACall()
-  }
-
+private class MarkedStep extends TaintTracking::AdditionalTaintStep {
   override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-    succ = this and
-    pred = this.getArgument(0)
+    exists(DataFlow::CallNode call |
+      call = DataFlow::globalVarRef("marked").getACall()
+      or
+      call = DataFlow::moduleImport("marked").getACall()
+    |
+      succ = call and
+      pred = call.getArgument(0)
+    )
   }
 }
 
 /**
  * A taint step for the `markdown-table` library.
  */
-private class MarkdownTableStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
-  MarkdownTableStep() { this = DataFlow::moduleImport("markdown-table").getACall() }
-
+private class MarkdownTableStep extends TaintTracking::AdditionalTaintStep {
   override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-    succ = this and
-    pred = this.getArgument(0)
+    exists(DataFlow::CallNode call | call = DataFlow::moduleImport("markdown-table").getACall() |
+      succ = call and
+      pred = call.getArgument(0)
+    )
   }
 }
 
 /**
  * A taint step for the `showdown` library.
  */
-private class ShowDownStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
-  ShowDownStep() {
-    this =
-      [DataFlow::globalVarRef("showdown"), DataFlow::moduleImport("showdown")]
-          .getAConstructorInvocation("Converter")
-          .getAMemberCall(["makeHtml", "makeMd"])
-  }
-
+private class ShowDownStep extends TaintTracking::AdditionalTaintStep {
   override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-    succ = this and
-    pred = this.getArgument(0)
+    exists(DataFlow::CallNode call |
+      call =
+        [DataFlow::globalVarRef("showdown"), DataFlow::moduleImport("showdown")]
+            .getAConstructorInvocation("Converter")
+            .getAMemberCall(["makeHtml", "makeMd"])
+    |
+      succ = call and
+      pred = call.getArgument(0)
+    )
   }
 }
 
@@ -93,16 +93,16 @@ private module Unified {
   /**
    * A taint step for the `unified` library.
    */
-  class UnifiedStep extends TaintTracking::AdditionalTaintStep, UnifiedChain {
-    UnifiedStep() {
-      // sanitizer. Mostly looking for `rehype-sanitize`, but also other plugins with `sanitize` in their name.
-      not this.getAUsedPlugin().getALocalSource() =
-        DataFlow::moduleImport(any(string s | s.matches("%sanitize%")))
-    }
-
+  class UnifiedStep extends TaintTracking::AdditionalTaintStep {
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = getInput() and
-      succ = getOutput()
+      exists(UnifiedChain chain |
+        // sanitizer. Mostly looking for `rehype-sanitize`, but also other plugins with `sanitize` in their name.
+        not chain.getAUsedPlugin().getALocalSource() =
+          DataFlow::moduleImport(any(string s | s.matches("%sanitize%")))
+      |
+        pred = chain.getInput() and
+        succ = chain.getOutput()
+      )
     }
   }
 }
@@ -110,11 +110,11 @@ private module Unified {
 /**
  * A taint step for the `snarkdown` library.
  */
-private class SnarkdownStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
-  SnarkdownStep() { this = DataFlow::moduleImport("snarkdown").getACall() }
-
+private class SnarkdownStep extends TaintTracking::AdditionalTaintStep {
   override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-    this = succ and
-    pred = this.getArgument(0)
+    exists(DataFlow::CallNode call | call = DataFlow::moduleImport("snarkdown").getACall() |
+      call = succ and
+      pred = call.getArgument(0)
+    )
   }
 }

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

@@ -281,74 +281,64 @@ module NodeJSLib {
   /**
    * A call to a path-module method that preserves taint.
    */
-  private class PathFlowTarget extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
-    DataFlow::Node tainted;
-
-    PathFlowTarget() {
-      exists(string methodName | this = NodeJSLib::Path::moduleMember(methodName).getACall() |
+  private class PathFlowTarget extends TaintTracking::AdditionalTaintStep {
+    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+      exists(DataFlow::CallNode call, string methodName |
+        call = NodeJSLib::Path::moduleMember(methodName).getACall() and succ = call
+      |
         // getters
-        methodName = "basename" and tainted = getArgument(0)
+        methodName = "basename" and pred = call.getArgument(0)
         or
-        methodName = "dirname" and tainted = getArgument(0)
+        methodName = "dirname" and pred = call.getArgument(0)
         or
-        methodName = "extname" and tainted = getArgument(0)
+        methodName = "extname" and pred = call.getArgument(0)
         or
         // transformers
-        methodName = "join" and tainted = getAnArgument()
+        methodName = "join" and pred = call.getAnArgument()
         or
-        methodName = "normalize" and tainted = getArgument(0)
+        methodName = "normalize" and pred = call.getArgument(0)
         or
-        methodName = "relative" and tainted = getArgument([0 .. 1])
+        methodName = "relative" and pred = call.getArgument([0 .. 1])
         or
-        methodName = "resolve" and tainted = getAnArgument()
+        methodName = "resolve" and pred = call.getAnArgument()
         or
-        methodName = "toNamespacedPath" and tainted = getArgument(0)
+        methodName = "toNamespacedPath" and pred = call.getArgument(0)
       )
     }
-
-    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = tainted and succ = this
-    }
   }
 
   /**
    * A call to a fs-module method that preserves taint.
    */
   private class FsFlowTarget extends TaintTracking::AdditionalTaintStep {
-    DataFlow::Node tainted;
-
-    FsFlowTarget() {
+    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
       exists(DataFlow::CallNode call, string methodName |
         call = FS::moduleMember(methodName).getACall()
       |
         methodName = "realpathSync" and
-        tainted = call.getArgument(0) and
-        this = call
+        pred = call.getArgument(0) and
+        succ = call
         or
         methodName = "realpath" and
-        tainted = call.getArgument(0) and
-        this = call.getCallback(1).getParameter(1)
+        pred = call.getArgument(0) and
+        succ = call.getCallback(1).getParameter(1)
       )
     }
-
-    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = tainted and succ = this
-    }
   }
 
   /**
    * A model of taint propagation through `new Buffer` and `Buffer.from`.
    */
-  private class BufferTaintStep extends TaintTracking::AdditionalTaintStep, DataFlow::InvokeNode {
-    BufferTaintStep() {
-      this = DataFlow::globalVarRef("Buffer").getAnInstantiation()
-      or
-      this = DataFlow::globalVarRef("Buffer").getAMemberInvocation("from")
-    }
-
+  private class BufferTaintStep extends TaintTracking::AdditionalTaintStep {
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = getArgument(0) and
-      succ = this
+      exists(DataFlow::InvokeNode invoke |
+        invoke = DataFlow::globalVarRef("Buffer").getAnInstantiation()
+        or
+        invoke = DataFlow::globalVarRef("Buffer").getAMemberInvocation("from")
+      |
+        pred = invoke.getArgument(0) and
+        succ = invoke
+      )
     }
   }
 

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

@@ -132,13 +132,8 @@ private class SimplePropertyProjection extends PropertyProjection::Range {
  * A taint step for a property projection.
  */
 private class PropertyProjectionTaintStep extends TaintTracking::AdditionalTaintStep {
-  PropertyProjection projection;
-
-  PropertyProjectionTaintStep() { projection = this }
-
   override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
     // reading from a tainted object yields a tainted result
-    this = succ and
-    pred = projection.getObject()
+    pred = succ.(PropertyProjection).getObject()
   }
 }

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

@@ -7,12 +7,12 @@ private import javascript
 /**
  * A step `x -> y` in `x.subscribe(y => ...)`, modeling flow out of an rxjs Observable.
  */
-private class RxJsSubscribeStep extends TaintTracking::AdditionalTaintStep, DataFlow::MethodCallNode {
-  RxJsSubscribeStep() { getMethodName() = "subscribe" }
-
+private class RxJsSubscribeStep extends TaintTracking::AdditionalTaintStep {
   override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-    pred = getReceiver() and
-    succ = getCallback(0).getParameter(0)
+    exists(DataFlow::MethodCallNode call | call.getMethodName() = "subscribe" |
+      pred = call.getReceiver() and
+      succ = call.getCallback(0).getParameter(0)
+    )
   }
 }
 
@@ -54,24 +54,24 @@ private predicate isIdentityPipe(DataFlow::CallNode pipe) {
 /**
  * A step in or out of the map callback in a call of form `x.pipe(map(y => ...))`.
  */
-private class RxJsPipeMapStep extends TaintTracking::AdditionalTaintStep, DataFlow::MethodCallNode {
-  RxJsPipeMapStep() { getMethodName() = "pipe" }
-
+private class RxJsPipeMapStep extends TaintTracking::AdditionalTaintStep {
   override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-    pred = getReceiver() and
-    succ = pipeInput(getArgument(0).getALocalSource())
-    or
-    exists(int i |
-      pred = pipeOutput(getArgument(i).getALocalSource()) and
-      succ = pipeInput(getArgument(i + 1).getALocalSource())
+    exists(DataFlow::MethodCallNode call | call.getMethodName() = "pipe" |
+      pred = call.getReceiver() and
+      succ = pipeInput(call.getArgument(0).getALocalSource())
+      or
+      exists(int i |
+        pred = pipeOutput(call.getArgument(i).getALocalSource()) and
+        succ = pipeInput(call.getArgument(i + 1).getALocalSource())
+      )
+      or
+      pred = pipeOutput(call.getLastArgument().getALocalSource()) and
+      succ = call
+      or
+      // Handle a common case where the last step is `catchError`.
+      isIdentityPipe(call.getLastArgument().getALocalSource()) and
+      pred = pipeOutput(call.getArgument(call.getNumArgument() - 2)) and
+      succ = call
     )
-    or
-    pred = pipeOutput(getLastArgument().getALocalSource()) and
-    succ = this
-    or
-    // Handle a common case where the last step is `catchError`.
-    isIdentityPipe(getLastArgument().getALocalSource()) and
-    pred = pipeOutput(getArgument(getNumArgument() - 2)) and
-    succ = this
   }
 }

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

@@ -126,11 +126,13 @@ module Typeahead {
   /**
    * A taint step that models that a function in the `source` of typeahead.js is used to determine the input to the suggestion function.
    */
-  private class TypeaheadSourceTaintStep extends TypeaheadSource, TaintTracking::AdditionalTaintStep {
+  private class TypeaheadSourceTaintStep extends TaintTracking::AdditionalTaintStep {
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
       // Matches `$(...).typeahead({..}, {source: function(q, cb) {..;cb(<pred>);..}, templates: { suggestion: function(<succ>) {} } })`.
-      pred = this.getAFunctionValue().getParameter([1 .. 2]).getACall().getAnArgument() and
-      succ = this.getASuggestion()
+      exists(TypeaheadSource source |
+        pred = source.getAFunctionValue().getParameter([1 .. 2]).getACall().getAnArgument() and
+        succ = source.getASuggestion()
+      )
     }
   }
 }

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

@@ -4,10 +4,22 @@
 
 import javascript
 
+private newtype TUnit = TUnitInjector()
+
+private class UriLibraryStepGlue extends TaintTracking::AdditionalTaintStep {
+  override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+    any(UriLibraryStep step).step(pred, succ)
+  }
+}
+
 /**
  * A taint propagating data flow edge arising from an operation in a URI library.
  */
-abstract class UriLibraryStep extends DataFlow::ValueNode, TaintTracking::AdditionalTaintStep { }
+class UriLibraryStep extends TUnit {
+  abstract predicate step(DataFlow::Node pred, DataFlow::Node succ);
+
+  string toString() { result = "Additional URI library taint step class" }
+}
 
 /**
  * Provides classes for working with [urijs](http://medialize.github.io/URI.js/) code.
@@ -57,25 +69,25 @@ module urijs {
    * A taint step in the urijs library.
    */
   private class Step extends UriLibraryStep {
-    DataFlow::Node src;
-
-    Step() {
-      // flow through "constructors" (`new` is optional)
-      exists(DataFlow::InvokeNode invk | invk = this and invk = invocation() |
-        src = invk.getAnArgument()
+    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+      exists(DataFlow::ValueNode value | value = succ |
+        // flow through "constructors" (`new` is optional)
+        exists(DataFlow::InvokeNode invk | invk = value and invk = invocation() |
+          pred = invk.getAnArgument()
+        )
+        or
+        // flow through chained calls
+        exists(DataFlow::MethodCallNode mc | mc = value and value = chainCall() |
+          pred = mc.getReceiver() or
+          pred = mc.getAnArgument()
+        )
+        or
+        // flow through getter calls
+        exists(DataFlow::MethodCallNode mc | mc = value and value = getter() |
+          pred = mc.getReceiver()
+        )
       )
-      or
-      // flow through chained calls
-      exists(DataFlow::MethodCallNode mc | mc = this and this = chainCall() |
-        src = mc.getReceiver() or
-        src = mc.getAnArgument()
-      )
-      or
-      // flow through getter calls
-      exists(DataFlow::MethodCallNode mc | mc = this and this = getter() | src = mc.getReceiver())
     }
-
-    override predicate step(DataFlow::Node pred, DataFlow::Node succ) { pred = src and succ = this }
   }
 }
 
@@ -93,22 +105,20 @@ module uridashjs {
   /**
    * A taint step in the urijs library.
    */
-  private class Step extends UriLibraryStep, DataFlow::CallNode {
-    DataFlow::Node src;
-
-    Step() {
-      exists(string name |
+  private class Step extends UriLibraryStep {
+    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+      exists(DataFlow::CallNode call, string name |
         name = "parse" or
         name = "serialize" or
         name = "resolve" or
         name = "normalize"
       |
-        this = uridashjsMember(name).getACall() and
-        src = getAnArgument()
+        call instanceof DataFlow::ValueNode and
+        call = succ and
+        call = uridashjsMember(name).getACall() and
+        pred = call.getAnArgument()
       )
     }
-
-    override predicate step(DataFlow::Node pred, DataFlow::Node succ) { pred = src and succ = this }
   }
 }
 
@@ -126,22 +136,20 @@ module punycode {
   /**
    * A taint step in the punycode library.
    */
-  private class Step extends UriLibraryStep, DataFlow::CallNode {
-    DataFlow::Node src;
-
-    Step() {
-      exists(string name |
+  private class Step extends UriLibraryStep {
+    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+      exists(DataFlow::CallNode call, string name |
         name = "decode" or
         name = "encode" or
         name = "toUnicode" or
         name = "toASCII"
       |
-        this = punycodeMember(name).getACall() and
-        src = getAnArgument()
+        call instanceof DataFlow::ValueNode and
+        call = succ and
+        call = punycodeMember(name).getACall() and
+        pred = call.getAnArgument()
       )
     }
-
-    override predicate step(DataFlow::Node pred, DataFlow::Node succ) { pred = src and succ = this }
   }
 }
 
@@ -162,24 +170,23 @@ module urlParse {
   /**
    * A taint step in the url-parse library.
    */
-  private class Step extends UriLibraryStep, DataFlow::CallNode {
-    DataFlow::Node src;
-
-    Step() {
+  private class Step extends UriLibraryStep {
+    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
       // parse(src)
-      this = call() and
-      src = getAnArgument()
+      succ = call() and
+      succ instanceof DataFlow::ValueNode and
+      pred = succ.(DataFlow::CallNode).getAnArgument()
       or
-      exists(DataFlow::MethodCallNode mc | this = mc and mc = call().getAMethodCall("set") |
+      exists(DataFlow::MethodCallNode mc |
+        mc instanceof DataFlow::ValueNode and succ = mc and mc = call().getAMethodCall("set")
+      |
         // src = parse(...); src.set(x, y)
-        src = mc.getReceiver()
+        pred = mc.getReceiver()
         or
         // parse(x).set(y, src)
-        src = mc.getArgument(1)
+        pred = mc.getArgument(1)
       )
     }
-
-    override predicate step(DataFlow::Node pred, DataFlow::Node succ) { pred = src and succ = this }
   }
 }
 
@@ -197,20 +204,18 @@ module querystringify {
   /**
    * A taint step in the querystringify library.
    */
-  private class Step extends UriLibraryStep, DataFlow::CallNode {
-    DataFlow::Node src;
-
-    Step() {
-      exists(string name |
+  private class Step extends UriLibraryStep {
+    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+      exists(DataFlow::CallNode call, string name |
         name = "parse" or
         name = "stringify"
       |
-        this = querystringifyMember(name).getACall() and
-        src = getAnArgument()
+        call = querystringifyMember(name).getACall() and
+        pred = call.getAnArgument() and
+        succ = call and
+        call instanceof DataFlow::ValueNode
       )
     }
-
-    override predicate step(DataFlow::Node pred, DataFlow::Node succ) { pred = src and succ = this }
   }
 }
 
@@ -228,22 +233,20 @@ module querydashstring {
   /**
    * A taint step in the query-string library.
    */
-  private class Step extends UriLibraryStep, DataFlow::CallNode {
-    DataFlow::Node src;
-
-    Step() {
-      exists(string name |
+  private class Step extends UriLibraryStep {
+    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+      exists(DataFlow::CallNode call, string name |
         name = "parse" or
         name = "extract" or
         name = "parseUrl" or
         name = "stringify"
       |
-        this = querydashstringMember(name).getACall() and
-        src = getAnArgument()
+        call = querydashstringMember(name).getACall() and
+        pred = call.getAnArgument() and
+        call instanceof DataFlow::ValueNode and
+        call = succ
       )
     }
-
-    override predicate step(DataFlow::Node pred, DataFlow::Node succ) { pred = src and succ = this }
   }
 }
 
@@ -259,21 +262,19 @@ module url {
   /**
    * A taint step in the url library.
    */
-  private class Step extends UriLibraryStep, DataFlow::CallNode {
-    DataFlow::Node src;
-
-    Step() {
-      exists(string name |
+  private class Step extends UriLibraryStep {
+    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+      exists(DataFlow::CallNode call, string name |
         name = "parse" or
         name = "format" or
         name = "resolve"
       |
-        this = urlMember(name).getACall() and
-        src = getAnArgument()
+        call = urlMember(name).getACall() and
+        call instanceof DataFlow::ValueNode and
+        succ = call and
+        pred = call.getAnArgument()
       )
     }
-
-    override predicate step(DataFlow::Node pred, DataFlow::Node succ) { pred = src and succ = this }
   }
 }
 
@@ -291,22 +292,20 @@ module querystring {
   /**
    * A taint step in the querystring library.
    */
-  private class Step extends UriLibraryStep, DataFlow::CallNode {
-    DataFlow::Node src;
-
-    Step() {
-      exists(string name |
+  private class Step extends UriLibraryStep {
+    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+      exists(DataFlow::CallNode call, string name |
         name = "escape" or
         name = "unescape" or
         name = "parse" or
         name = "stringify"
       |
-        this = querystringMember(name).getACall() and
-        src = getAnArgument()
+        call = querystringMember(name).getACall() and
+        call instanceof DataFlow::ValueNode and
+        call = succ and
+        pred = call.getAnArgument()
       )
     }
-
-    override predicate step(DataFlow::Node pred, DataFlow::Node succ) { pred = src and succ = this }
   }
 }
 
@@ -317,56 +316,55 @@ private module ClosureLibraryUri {
   /**
    * Taint step from an argument of a `goog.Uri` call to the return value.
    */
-  private class ArgumentStep extends UriLibraryStep, DataFlow::InvokeNode {
-    int arg;
-
-    ArgumentStep() {
-      // goog.Uri constructor
-      this = Closure::moduleImport("goog.Uri").getAnInstantiation() and arg = 0
-      or
-      // static methods on goog.Uri
-      exists(string name | this = Closure::moduleImport("goog.Uri." + name).getACall() |
-        name = "parse" and arg = 0
-        or
-        name = "create" and
-        (arg = 0 or arg = 2 or arg = 4)
-        or
-        name = "resolve" and
-        (arg = 0 or arg = 1)
-      )
-      or
-      // static methods in goog.uri.utils
-      arg = 0 and
-      exists(string name | this = Closure::moduleImport("goog.uri.utils." + name).getACall() |
-        name = "appendParam" or // preserve taint from the original URI, but not from the appended param
-        name = "appendParams" or
-        name = "appendParamsFromMap" or
-        name = "appendPath" or
-        name = "getParamValue" or
-        name = "getParamValues" or
-        name = "getPath" or
-        name = "getPathAndAfter" or
-        name = "getQueryData" or
-        name = "parseQueryData" or
-        name = "removeFragment" or
-        name = "removeParam" or
-        name = "setParam" or
-        name = "setParamsFromMap" or
-        name = "setPath" or
-        name = "split"
-      )
-      or
-      // static methods in goog.string
-      arg = 0 and
-      exists(string name | this = Closure::moduleImport("goog.string." + name).getACall() |
-        name = "urlDecode" or
-        name = "urlEncode"
-      )
-    }
-
+  private class ArgumentStep extends UriLibraryStep {
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = getArgument(arg) and
-      succ = this
+      exists(DataFlow::InvokeNode invoke, int arg |
+        // goog.Uri constructor
+        invoke = Closure::moduleImport("goog.Uri").getAnInstantiation() and arg = 0
+        or
+        // static methods on goog.Uri
+        exists(string name | invoke = Closure::moduleImport("goog.Uri." + name).getACall() |
+          name = "parse" and arg = 0
+          or
+          name = "create" and
+          (arg = 0 or arg = 2 or arg = 4)
+          or
+          name = "resolve" and
+          (arg = 0 or arg = 1)
+        )
+        or
+        // static methods in goog.uri.utils
+        arg = 0 and
+        exists(string name | invoke = Closure::moduleImport("goog.uri.utils." + name).getACall() |
+          name = "appendParam" or // preserve taint from the original URI, but not from the appended param
+          name = "appendParams" or
+          name = "appendParamsFromMap" or
+          name = "appendPath" or
+          name = "getParamValue" or
+          name = "getParamValues" or
+          name = "getPath" or
+          name = "getPathAndAfter" or
+          name = "getQueryData" or
+          name = "parseQueryData" or
+          name = "removeFragment" or
+          name = "removeParam" or
+          name = "setParam" or
+          name = "setParamsFromMap" or
+          name = "setPath" or
+          name = "split"
+        )
+        or
+        // static methods in goog.string
+        arg = 0 and
+        exists(string name | invoke = Closure::moduleImport("goog.string." + name).getACall() |
+          name = "urlDecode" or
+          name = "urlEncode"
+        )
+      |
+        pred = invoke.getArgument(arg) and
+        succ = invoke and
+        succ instanceof DataFlow::ValueNode
+      )
     }
   }
 
@@ -375,7 +373,7 @@ private module ClosureLibraryUri {
    *
    * Setters mutate the URI object and return the same instance.
    */
-  private class SetterCall extends DataFlow::MethodCallNode, UriLibraryStep {
+  private class SetterCall extends DataFlow::MethodCallNode {
     DataFlow::NewNode uri;
     string name;
 
@@ -391,14 +389,20 @@ private module ClosureLibraryUri {
       )
     }
 
-    DataFlow::NewNode getUri() { result = uri }
+    string getName() { result = name }
 
+    DataFlow::NewNode getUri() { result = uri }
+  }
+
+  private class SetterCallStep extends UriLibraryStep {
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = getReceiver() and succ = this
-      or
-      (name = "setDomain" or name = "setPath" or name = "setScheme") and
-      pred = getArgument(0) and
-      succ = uri
+      exists(SetterCall setterCall |
+        pred = setterCall.getReceiver() and succ = setterCall
+        or
+        setterCall.getName() = ["setDomain", "setPath", "setScheme"] and
+        pred = setterCall.getArgument(0) and
+        succ = setterCall.getUri()
+      )
     }
   }
 
@@ -409,25 +413,21 @@ private module ClosureLibraryUri {
     /**
      * A taint step in the path module.
      */
-    private class Step extends UriLibraryStep, DataFlow::CallNode {
-      DataFlow::Node src;
-
-      Step() {
-        exists(DataFlow::SourceNode ref |
+    private class Step extends UriLibraryStep {
+      override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+        exists(DataFlow::CallNode call, DataFlow::SourceNode ref |
           ref = NodeJSLib::Path::moduleMember("parse") or
           // a ponyfill: https://www.npmjs.com/package/path-parse
           ref = DataFlow::moduleImport("path-parse") or
           ref = DataFlow::moduleMember("path-parse", "posix") or
           ref = DataFlow::moduleMember("path-parse", "win32")
         |
-          this = ref.getACall() and
-          src = getAnArgument()
+          call = ref.getACall() and
+          call instanceof DataFlow::ValueNode and
+          call = succ and
+          pred = call.getAnArgument()
         )
       }
-
-      override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-        pred = src and succ = this
-      }
     }
   }
 }

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

@@ -476,18 +476,14 @@ module Vue {
    * A taint propagating data flow edge through a Vue instance property.
    */
   class InstanceHeapStep extends TaintTracking::AdditionalTaintStep {
-    DataFlow::Node src;
-
-    InstanceHeapStep() {
+    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
       exists(Instance i, string name, DataFlow::FunctionNode bound |
         bound.flowsTo(i.getABoundFunction()) and
         not bound.getFunction() instanceof ArrowFunctionExpr and
-        bound.getReceiver().getAPropertyRead(name) = this and
-        src = i.getAPropertyValue(name)
+        bound.getReceiver().getAPropertyRead(name) = succ and
+        pred = i.getAPropertyValue(name)
       )
     }
-
-    override predicate step(DataFlow::Node pred, DataFlow::Node succ) { pred = src and succ = this }
   }
 
   /*

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

@@ -277,13 +277,11 @@ module XML {
   }
 
   private class XMLParserTaintStep extends js::TaintTracking::AdditionalTaintStep {
-    XML::ParserInvocation parser;
-
-    XMLParserTaintStep() { this.asExpr() = parser }
-
     override predicate step(js::DataFlow::Node pred, js::DataFlow::Node succ) {
-      pred.asExpr() = parser.getSourceArgument() and
-      succ = parser.getAResult()
+      exists(XML::ParserInvocation parser |
+        pred.asExpr() = parser.getSourceArgument() and
+        succ = parser.getAResult()
+      )
     }
   }
 }

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

@@ -656,7 +656,7 @@ module TaintedPath {
     or
     promiseTaintStep(src, dst) and srclabel = dstlabel
     or
-    any(TaintTracking::PersistentStorageTaintStep st).step(src, dst) and srclabel = dstlabel
+    TaintTracking::persistentStorageTaintStep(src, dst) and srclabel = dstlabel
     or
     exists(DataFlow::PropRead read | read = dst |
       src = read.getBase() and

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

@@ -359,22 +359,17 @@ module DomBasedXss {
    * `div` element is part of the template for `inst`.
    */
   class VHtmlSourceWrite extends TaintTracking::AdditionalTaintStep {
-    VHtmlSink attr;
-
-    VHtmlSourceWrite() {
-      exists(Vue::Instance instance, string expr |
+    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+      exists(VHtmlSink attr, Vue::Instance instance, string expr |
         attr.getAttr().getRoot() =
           instance.getTemplateElement().(Vue::Template::HtmlElement).getElement() and
         expr = attr.getAttr().getValue() and
         // only support for simple identifier expressions
         expr.regexpMatch("(?i)[a-z0-9_]+") and
-        this = instance.getAPropertyValue(expr)
+        pred = instance.getAPropertyValue(expr) and
+        succ = attr
       )
     }
-
-    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = this and succ = attr
-    }
   }
 
   /**

javascript/ql/test/library-tests/frameworks/Redux/exportedReducer.js

@@ -0,0 +1,13 @@
+import { combineReducers } from 'redux';
+
+export default (state, action) => {
+    return state;
+};
+
+export function notAReducer(notState, notAction) {
+    console.log(notState, notAction);
+}
+
+export const nestedReducer = combineReducers({
+    inner: (state, action) => state
+});

javascript/ql/test/library-tests/frameworks/Redux/react-redux.jsx

@@ -0,0 +1,99 @@
+import * as React from 'react';
+import { connect, useDispatch } from 'react-redux';
+import * as rt from '@reduxjs/toolkit';
+
+const toolkitAction = rt.createAction('toolkitAction', (x) => {
+    return {
+        toolkitValue: x
+    }
+});
+const toolkitReducer = rt.createReducer({}, builder => {
+    builder
+        .addCase(toolkitAction, (state, action) => {
+            return {
+                value: action.payload.toolkitValue,
+                ...state
+            };
+        })
+        .addCase(asyncAction.fulfilled, (state, action) => {
+            return {
+                asyncValue: action.payload.x,
+                ...state
+            };
+        });
+});
+
+function manualAction(x) {
+    return {
+        type: 'manualAction',
+        payload: x
+    }
+}
+function manualReducer(state, action) {
+    switch (action.type) {
+        case 'manualAction': {
+            return { ...state, manualValue: action.payload };
+        }
+    }
+    if (action.type === 'manualAction') {
+        return { ...state, manualValue2: action.payload };
+    }
+    if (action.type === 'manualAction') {
+        return {
+            ...state,
+            manualValue3: [1, 2, 3].map(x => {
+                return action.payload + x;
+            })
+        };
+    }
+    return state;
+}
+const asyncAction = rt.createAsyncThunk('asyncAction', (x) => {
+    return new Promise((resolve) => {
+        setTimeout(() => {
+            resolve({ x });
+        }, 10)
+    });
+});
+
+const store = rt.createStore(rt.combineReducers({
+    toolkit: toolkitReducer,
+    manual: manualReducer,
+}));
+
+function MyComponent(props) {
+    let dispatch = useDispatch();
+    const clickHandler = React.useCallback(() => {
+        props.toolkitAction(source());
+        props.manualAction(source()); // not currently propagated as functions are not type-tracked
+        dispatch(manualAction(source()));
+        dispatch(asyncAction(source()));
+    });
+
+    sink(props.propFromToolkitAction); // NOT OK
+    sink(props.propFromManualAction); // NOT OK
+    sink(props.propFromManualAction2); // NOT OK
+    sink(props.propFromManualAction3); // NOT OK
+    sink(props.propFromAsync); // NOT OK
+
+    return <button onClick={{clickHandler}}/>
+}
+
+function mapStateToProps(state) {
+    return {
+        propFromToolkitAction: state.toolkit.value,
+        propFromAsync: state.toolkit.asyncValue,
+        propFromManualAction: state.manual.manualValue,
+        propFromManualAction2: state.manual.manualValue2,
+        propFromManualAction3: state.manual.manualValue3,
+    }
+}
+
+const mapDispatchToProps = { toolkitAction, manualAction };
+
+const ConnectedComponent = connect(mapStateToProps, mapDispatchToProps)(MyComponent);
+
+function connectLike(f, g) {
+    return c => somethingWeirdAndComplicated(f, g)(c);
+}
+const ConnectedComponent2 = connectLike(mapStateToProps, mapDispatchToProps)(MyComponent);

javascript/ql/test/library-tests/frameworks/Redux/test.expected

@@ -0,0 +1,165 @@
+reducerArg
+| exportedReducer.js:12:12:12:35 | (state, ... > state |
+| react-redux.jsx:12:33:17:9 | (state, ...       } |
+| react-redux.jsx:18:41:23:9 | (state, ...       } |
+| react-redux.jsx:59:30:62:2 | rt.comb ... cer,\\n}) |
+| react-redux.jsx:60:14:60:27 | toolkitReducer |
+| react-redux.jsx:61:13:61:25 | manualReducer |
+| trivial.js:10:10:10:33 | (state, ... > state |
+| trivial.js:11:10:13:5 | {\\n      ... ,\\n    } |
+| trivial.js:12:14:12:37 | (state, ... > state |
+| trivial.js:16:10:16:33 | (state, ... > state |
+| trivial.js:17:10:19:5 | {\\n      ... ,\\n    } |
+| trivial.js:18:14:18:37 | (state, ... > state |
+| trivial.js:22:10:22:33 | (state, ... > state |
+| trivial.js:23:10:25:5 | {\\n      ... ,\\n    } |
+| trivial.js:24:14:24:37 | (state, ... > state |
+| trivial.js:29:16:29:39 | (state, ... > state |
+| trivial.js:32:73:32:96 | (state, ... > state |
+| trivial.js:46:33:46:56 | (state, ... > state |
+| trivial.js:47:33:47:56 | (state, ... > state |
+| trivial.js:130:14:130:46 | wrapper ...  state) |
+| trivial.js:133:45:133:66 | combine ... Reducer |
+| trivial.js:134:56:134:79 | (state, ... > state |
+| trivial.js:136:14:136:37 | (state, ... > state |
+isActionTypeHandler
+| react-redux.jsx:12:18:12:30 | toolkitAction | react-redux.jsx:12:33:17:9 | (state, ...       } |
+| react-redux.jsx:18:18:18:38 | asyncAc ... lfilled | react-redux.jsx:18:41:23:9 | (state, ...       } |
+| trivial.js:29:5:29:13 | fooAction | trivial.js:29:16:29:39 | (state, ... > state |
+| trivial.js:32:60:32:70 | 'fooAction' | trivial.js:32:73:32:96 | (state, ... > state |
+| trivial.js:46:18:46:30 | toolkitAction | trivial.js:46:33:46:56 | (state, ... > state |
+| trivial.js:47:18:47:30 | toolkitAction | trivial.js:47:33:47:56 | (state, ... > state |
+isTypeTagHandler
+| asyncAction | react-redux.jsx:18:41:23:9 | (state, ...       } |
+| counter/increment | trivial.js:46:33:46:56 | (state, ... > state |
+| counter/increment | trivial.js:47:33:47:56 | (state, ... > state |
+| fooAction | trivial.js:29:16:29:39 | (state, ... > state |
+| fooAction | trivial.js:32:73:32:96 | (state, ... > state |
+| toolkitAction | react-redux.jsx:12:33:17:9 | (state, ...       } |
+isRootStateHandler
+| react-redux.jsx:59:30:62:2 | rt.comb ... cer,\\n}) |
+| trivial.js:133:45:133:66 | combine ... Reducer |
+| trivial.js:134:56:134:79 | (state, ... > state |
+| trivial.js:136:14:136:37 | (state, ... > state |
+delegatingReducer
+| exportedReducer.js:11:30:13:2 | combine ... tate\\n}) |
+| react-redux.jsx:10:24:24:2 | rt.crea ...  });\\n}) |
+| react-redux.jsx:59:30:62:2 | rt.comb ... cer,\\n}) |
+| trivial.js:9:22:14:2 | require ...    }\\n}) |
+| trivial.js:11:10:13:5 | {\\n      ... ,\\n    } |
+| trivial.js:15:26:20:2 | require ...    }\\n}) |
+| trivial.js:17:10:19:5 | {\\n      ... ,\\n    } |
+| trivial.js:21:24:26:2 | require ...    }\\n}) |
+| trivial.js:23:10:25:5 | {\\n      ... ,\\n    } |
+| trivial.js:28:23:30:2 | require ... ate,\\n}) |
+| trivial.js:32:22:32:97 | require ...  state) |
+| trivial.js:34:24:34:88 | require ...  state) |
+| trivial.js:36:15:36:56 | require ...  state) |
+| trivial.js:37:22:37:71 | require ...  state) |
+| trivial.js:39:25:39:76 | require ...  state) |
+| trivial.js:40:25:40:96 | require ... cers1]) |
+| trivial.js:41:25:41:89 | require ...  state) |
+| trivial.js:42:25:42:109 | require ... cers1]) |
+| trivial.js:44:23:48:2 | require ... ate)\\n}) |
+| trivial.js:129:32:131:2 | require ... te),\\n}) |
+getStateHandlerArg
+| exportedReducer.js:11:30:13:2 | combine ... tate\\n}) | inner | exportedReducer.js:12:12:12:35 | (state, ... > state |
+| react-redux.jsx:59:30:62:2 | rt.comb ... cer,\\n}) | manual | react-redux.jsx:61:13:61:25 | manualReducer |
+| react-redux.jsx:59:30:62:2 | rt.comb ... cer,\\n}) | toolkit | react-redux.jsx:60:14:60:27 | toolkitReducer |
+| trivial.js:9:22:14:2 | require ...    }\\n}) | bar | trivial.js:11:10:13:5 | {\\n      ... ,\\n    } |
+| trivial.js:9:22:14:2 | require ...    }\\n}) | foo | trivial.js:10:10:10:33 | (state, ... > state |
+| trivial.js:11:10:13:5 | {\\n      ... ,\\n    } | baz | trivial.js:12:14:12:37 | (state, ... > state |
+| trivial.js:15:26:20:2 | require ...    }\\n}) | bar | trivial.js:17:10:19:5 | {\\n      ... ,\\n    } |
+| trivial.js:15:26:20:2 | require ...    }\\n}) | foo | trivial.js:16:10:16:33 | (state, ... > state |
+| trivial.js:17:10:19:5 | {\\n      ... ,\\n    } | baz | trivial.js:18:14:18:37 | (state, ... > state |
+| trivial.js:21:24:26:2 | require ...    }\\n}) | bar | trivial.js:23:10:25:5 | {\\n      ... ,\\n    } |
+| trivial.js:21:24:26:2 | require ...    }\\n}) | foo | trivial.js:22:10:22:33 | (state, ... > state |
+| trivial.js:23:10:25:5 | {\\n      ... ,\\n    } | baz | trivial.js:24:14:24:37 | (state, ... > state |
+| trivial.js:129:32:131:2 | require ... te),\\n}) | wrapped | trivial.js:130:14:130:46 | wrapper ...  state) |
+getActionHandlerArg
+| react-redux.jsx:10:24:24:2 | rt.crea ...  });\\n}) | react-redux.jsx:12:18:12:30 | toolkitAction | react-redux.jsx:12:33:17:9 | (state, ...       } |
+| react-redux.jsx:10:24:24:2 | rt.crea ...  });\\n}) | react-redux.jsx:18:18:18:38 | asyncAc ... lfilled | react-redux.jsx:18:41:23:9 | (state, ...       } |
+| trivial.js:28:23:30:2 | require ... ate,\\n}) | trivial.js:29:5:29:13 | fooAction | trivial.js:29:16:29:39 | (state, ... > state |
+| trivial.js:32:22:32:97 | require ...  state) | trivial.js:32:60:32:70 | 'fooAction' | trivial.js:32:73:32:96 | (state, ... > state |
+| trivial.js:44:23:48:2 | require ... ate)\\n}) | trivial.js:46:18:46:30 | toolkitAction | trivial.js:46:33:46:56 | (state, ... > state |
+| trivial.js:44:23:48:2 | require ... ate)\\n}) | trivial.js:47:18:47:30 | toolkitAction | trivial.js:47:33:47:56 | (state, ... > state |
+getAPlainHandlerArg
+| trivial.js:36:15:36:56 | require ...  state) | trivial.js:36:32:36:55 | (state, ... > state |
+| trivial.js:37:22:37:71 | require ...  state) | trivial.js:37:47:37:70 | (state, ... > state |
+| trivial.js:39:25:39:76 | require ...  state) | trivial.js:39:52:39:75 | (state, ... > state |
+| trivial.js:40:25:40:96 | require ... cers1]) | trivial.js:40:52:40:95 | [(state ... ucers1] |
+| trivial.js:40:25:40:96 | require ... cers1]) | trivial.js:40:53:40:76 | (state, ... > state |
+| trivial.js:40:25:40:96 | require ... cers1]) | trivial.js:40:79:40:94 | reducerReducers1 |
+| trivial.js:41:25:41:89 | require ...  state) | trivial.js:41:65:41:88 | (state, ... > state |
+| trivial.js:42:25:42:109 | require ... cers1]) | trivial.js:42:65:42:108 | [(state ... ucers1] |
+| trivial.js:42:25:42:109 | require ... cers1]) | trivial.js:42:66:42:89 | (state, ... > state |
+| trivial.js:42:25:42:109 | require ... cers1]) | trivial.js:42:92:42:107 | reducerReducers1 |
+getUseSite
+| react-redux.jsx:10:24:24:2 | rt.crea ...  });\\n}) | react-redux.jsx:60:14:60:27 | toolkitReducer |
+| react-redux.jsx:59:30:62:2 | rt.comb ... cer,\\n}) | react-redux.jsx:59:30:62:2 | rt.comb ... cer,\\n}) |
+| trivial.js:11:10:13:5 | {\\n      ... ,\\n    } | trivial.js:11:10:13:5 | {\\n      ... ,\\n    } |
+| trivial.js:17:10:19:5 | {\\n      ... ,\\n    } | trivial.js:17:10:19:5 | {\\n      ... ,\\n    } |
+| trivial.js:23:10:25:5 | {\\n      ... ,\\n    } | trivial.js:23:10:25:5 | {\\n      ... ,\\n    } |
+| trivial.js:129:32:131:2 | require ... te),\\n}) | trivial.js:133:45:133:66 | combine ... Reducer |
+storeCreation
+| react-redux.jsx:59:15:62:3 | rt.crea ... er,\\n})) |
+| trivial.js:133:16:133:67 | require ... educer) |
+| trivial.js:134:16:134:80 | require ...  state) |
+| trivial.js:135:16:137:2 | require ... tate\\n}) |
+taintFlow
+| react-redux.jsx:67:29:67:36 | source() | react-redux.jsx:73:10:73:36 | props.p ... tAction |
+| react-redux.jsx:69:31:69:38 | source() | react-redux.jsx:74:10:74:35 | props.p ... lAction |
+| react-redux.jsx:69:31:69:38 | source() | react-redux.jsx:75:10:75:36 | props.p ... Action2 |
+| react-redux.jsx:69:31:69:38 | source() | react-redux.jsx:76:10:76:36 | props.p ... Action3 |
+| react-redux.jsx:70:30:70:37 | source() | react-redux.jsx:77:10:77:28 | props.propFromAsync |
+reactComponentRef
+| react-redux.jsx:64:1:80:1 | functio ... r}}/>\\n} | react-redux.jsx:64:1:80:1 | functio ... r}}/>\\n} |
+| react-redux.jsx:64:1:80:1 | functio ... r}}/>\\n} | react-redux.jsx:94:28:94:84 | connect ... ponent) |
+| react-redux.jsx:64:1:80:1 | functio ... r}}/>\\n} | react-redux.jsx:97:12:97:12 | c |
+getAffectedStateAccessPath
+| react-redux.jsx:12:33:17:9 | (state, ...       } | toolkit |
+| react-redux.jsx:18:41:23:9 | (state, ...       } | toolkit |
+| react-redux.jsx:60:14:60:27 | toolkitReducer | toolkit |
+| react-redux.jsx:61:13:61:25 | manualReducer | manual |
+| trivial.js:130:14:130:46 | wrapper ...  state) | wrapped |
+getADispatchFunctionNode
+| react-redux.jsx:65:20:65:32 | use (return (member useDispatch (member exports (module react-redux)))) |
+getADispatchedValueNode
+| react-redux.jsx:27:12:30:5 | def (return (member manualAction (parameter 1 (react-redux-connect)))) |
+| react-redux.jsx:69:18:69:39 | def (parameter 0 (return (member useDispatch (member exports (module react-redux))))) |
+| react-redux.jsx:70:18:70:38 | def (parameter 0 (return (member useDispatch (member exports (module react-redux))))) |
+getAnUntypedActionInReducer
+| exportedReducer.js:12:20:12:25 | action |
+| react-redux.jsx:32:31:32:36 | action |
+| trivial.js:10:18:10:23 | action |
+| trivial.js:12:22:12:27 | action |
+| trivial.js:16:18:16:23 | action |
+| trivial.js:18:22:18:27 | action |
+| trivial.js:22:18:22:23 | action |
+| trivial.js:24:22:24:27 | action |
+| trivial.js:124:20:124:25 | action |
+| trivial.js:130:30:130:35 | action |
+| trivial.js:134:64:134:69 | action |
+| trivial.js:136:22:136:27 | action |
+actionToReducerStep
+| react-redux.jsx:5:56:9:1 | return of anonymous function | react-redux.jsx:14:24:14:37 | action.payload |
+| react-redux.jsx:29:18:29:18 | x | react-redux.jsx:35:45:35:58 | action.payload |
+| react-redux.jsx:29:18:29:18 | x | react-redux.jsx:39:42:39:55 | action.payload |
+| react-redux.jsx:29:18:29:18 | x | react-redux.jsx:45:24:45:37 | action.payload |
+| react-redux.jsx:67:29:67:36 | source() | react-redux.jsx:5:57:5:57 | x |
+| react-redux.jsx:70:30:70:37 | source() | react-redux.jsx:51:57:51:57 | x |
+actionToReducerPromiseStep
+| react-redux.jsx:51:56:57:1 | return of anonymous function | react-redux.jsx:20:29:20:42 | action.payload |
+reducerToStateStep
+| react-redux.jsx:12:33:17:9 | return of anonymous function | react-redux.jsx:84:32:84:44 | state.toolkit |
+| react-redux.jsx:12:33:17:9 | return of anonymous function | react-redux.jsx:85:24:85:36 | state.toolkit |
+| react-redux.jsx:14:24:14:50 | action. ... itValue | react-redux.jsx:84:32:84:50 | state.toolkit.value |
+| react-redux.jsx:18:41:23:9 | return of anonymous function | react-redux.jsx:84:32:84:44 | state.toolkit |
+| react-redux.jsx:18:41:23:9 | return of anonymous function | react-redux.jsx:85:24:85:36 | state.toolkit |
+| react-redux.jsx:20:29:20:44 | action.payload.x | react-redux.jsx:85:24:85:47 | state.t ... ncValue |
+| react-redux.jsx:32:1:50:1 | return of function manualReducer | react-redux.jsx:86:31:86:42 | state.manual |
+| react-redux.jsx:32:1:50:1 | return of function manualReducer | react-redux.jsx:87:32:87:43 | state.manual |
+| react-redux.jsx:32:1:50:1 | return of function manualReducer | react-redux.jsx:88:32:88:43 | state.manual |
+| react-redux.jsx:35:45:35:58 | action.payload | react-redux.jsx:86:31:86:54 | state.m ... alValue |
+| react-redux.jsx:39:42:39:55 | action.payload | react-redux.jsx:87:32:87:56 | state.m ... lValue2 |
+| react-redux.jsx:44:27:46:14 | [1, 2,  ...      }) | react-redux.jsx:88:32:88:56 | state.m ... lValue3 |

javascript/ql/test/library-tests/frameworks/Redux/test.ql

@@ -0,0 +1,65 @@
+import javascript
+
+query predicate getAffectedStateAccessPath = Redux::getAffectedStateAccessPath/1;
+
+query Redux::ReducerArg reducerArg() { any() }
+
+query Redux::ReducerArg isActionTypeHandler(DataFlow::Node actionType) {
+  result.isActionTypeHandler(actionType)
+}
+
+query Redux::ReducerArg isTypeTagHandler(string typeTag) { result.isTypeTagHandler(typeTag) }
+
+query Redux::ReducerArg isRootStateHandler() { result.isRootStateHandler() }
+
+query Redux::DelegatingReducer delegatingReducer() { any() }
+
+query DataFlow::Node getStateHandlerArg(Redux::DelegatingReducer reducer, string prop) {
+  result = reducer.getStateHandlerArg(prop)
+}
+
+query DataFlow::Node getActionHandlerArg(Redux::DelegatingReducer reducer, DataFlow::Node actionType) {
+  result = reducer.getActionHandlerArg(actionType)
+}
+
+query DataFlow::Node getAPlainHandlerArg(Redux::DelegatingReducer reducer) {
+  result = reducer.getAPlainHandlerArg()
+}
+
+query Redux::ReducerArg getUseSite(Redux::DelegatingReducer reducer) {
+  result = reducer.getUseSite()
+}
+
+query predicate getADispatchFunctionNode = Redux::getADispatchFunctionNode/0;
+
+query predicate getADispatchedValueNode = Redux::getADispatchedValueNode/0;
+
+query predicate getAnUntypedActionInReducer = Redux::getAnUntypedActionInReducer/0;
+
+query predicate actionToReducerStep = Redux::actionToReducerStep/2;
+
+query predicate actionToReducerPromiseStep = Redux::actionToReducerPromiseStep/2;
+
+query predicate reducerToStateStep = Redux::reducerToStateStep/2;
+
+query Redux::StoreCreation storeCreation() { any() }
+
+class BasicTaint extends TaintTracking::Configuration {
+  BasicTaint() { this = "BasicTaint" }
+
+  override predicate isSource(DataFlow::Node node) {
+    node.(DataFlow::CallNode).getCalleeName() = "source"
+  }
+
+  override predicate isSink(DataFlow::Node node) {
+    node = any(DataFlow::CallNode call | call.getCalleeName() = "sink").getAnArgument()
+  }
+}
+
+query predicate taintFlow(DataFlow::Node source, DataFlow::Node sink) {
+  any(BasicTaint cfg).hasFlow(source, sink)
+}
+
+query DataFlow::SourceNode reactComponentRef(ReactComponent component) {
+  result = component.getAComponentCreatorReference()
+}

javascript/ql/test/library-tests/frameworks/Redux/trivial.js

@@ -0,0 +1,137 @@
+// This file contains a lot of trivial reducers and actions, simply to test that
+// the Redux model recognizes them, but no data flow passes through them.
+
+const toolkitAction = require('@reduxjs/toolkit').createAction('counter/increment');
+const toolkitAsyncAction = require('@reduxjs/toolkit').createAsyncThunk('async-action', async (arg) => {
+    return await (await fetch(arg)).json();
+})
+
+const reduxCombine = require('redux').combineReducers({
+    foo: (state, action) => state,
+    bar: {
+        baz: (state, action) => state,
+    }
+});
+const immutableCombine = require('redux-immutable').combineReducers({
+    foo: (state, action) => state,
+    bar: {
+        baz: (state, action) => state,
+    }
+});
+const toolkitCombine = require('@reduxjs/toolkit').combineReducers({
+    foo: (state, action) => state,
+    bar: {
+        baz: (state, action) => state,
+    }
+});
+
+const handleActions = require('redux-actions').handleActions({
+    fooAction: (state, action) => state,
+});
+
+const handleAction = require('redux-actions').handleAction('fooAction', (state, action) => state);
+
+const persistReducer = require('redux-persist').persistReducer((state, action) => state);
+
+const immer = require('immer')((state, action) => state);
+const immerProduce = require('immer').produce((state, action) => state);
+
+const reduceReducers1 = require('reduce-reducers')((state, action) => state);
+const reduceReducers2 = require('reduce-reducers')([(state, action) => state, reducerReducers1]);
+const reduceReducers3 = require('redux-actions').reduceReducers((state, action) => state);
+const reduceReducers4 = require('redux-actions').reduceReducers([(state, action) => state, reducerReducers1]);
+
+const createReducer = require('@reduxjs/toolkit').createReducer(0, builder => {
+    builder
+        .addCase(toolkitAction, (state, action) => state)
+        .addCase(toolkitAction, (state, action) => state)
+});
+
+function createSlice1() {
+    const { increment } = require('@reduxjs/toolkit').createSlice({
+        name: 'counter1',
+        initialState: 0,
+        reducers: {
+            increment(state, action) {
+                return state;
+            }
+        },
+        extraReducers: {
+            [toolkitAction]: (state, action) => {
+                return state;
+            },
+            [toolkitAsyncAction.fulfilled]: (state, action) => {
+                action.meta.arg;
+                return state;
+            },
+            [toolkitAsyncAction.rejected]: (state, action) => {
+                action.meta.arg;
+                return state;
+            },
+            [toolkitAsyncAction.pending]: (state, action) => {
+                action.meta.arg;
+                return state;
+            }
+        }
+    });
+    return increment;
+}
+
+function createSlice2() {
+    const { increment } = require('@reduxjs/toolkit').createSlice({
+        name: 'counter2',
+        initialState: 0,
+        reducers: {
+            increment(state, action) {
+                return state;
+            }
+        },
+        extraReducers: builder => {
+            builder
+                .addCase(toolkitAction, (state, action) => state)
+                .addCase(toolkitAsyncAction.fulfilled, (state, action) => {
+                    action.meta.arg;
+                    return state;
+                });
+        }
+    });
+    return increment;
+}
+
+let importedReducers = combineReducers({
+    foo: {
+        bar: {
+            baz: (state, action) => state
+        },
+        baloon: require('./exportedReducer'),
+    },
+    nestedReducer: require('./exportedReducer').nestedReducer
+});
+
+const reduxActions = require('redux-actions').createAction('reduxActionsCreateAction');
+const tsUtils = require('redux-ts-utils').createAction('tsUtilCreateAction');
+
+const { fooAction2, barAction2 } = require('redux-actions').createActions({
+    foo1Action2(x, y) {
+        return { x, y }
+    },
+    barAction2(x) {
+        return { x }
+    }
+});
+
+function wrapper(fn) {
+    return (state, action) => {
+        console.log('hello');
+        return fn(state, action);
+    }
+}
+const combinedWrappedReducer = require('redux').combineReducers({
+    wrapped: wrapper((state, action) => state),
+});
+
+const store1 = require('redux').createStore(combinedWrappedReducer);
+const store2 = require('@reduxjs/toolkit').createStore((state, action) => state);
+const store3 = require('@reduxjs/toolkit').configureStore({
+    reducer: (state, action) => state
+});

javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query5.ql

@@ -1,11 +1,11 @@
 import javascript
 
-class StepThroughResolveSymlinks extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
-  StepThroughResolveSymlinks() { this = DataFlow::moduleImport("resolve-symlinks").getACall() }
-
+class StepThroughResolveSymlinks extends TaintTracking::AdditionalTaintStep {
   override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-    pred = this.getArgument(0) and
-    succ = this
+    exists(DataFlow::CallNode call | call = DataFlow::moduleImport("resolve-symlinks").getACall() |
+      pred = call.getArgument(0) and
+      succ = call
+    )
   }
 }