Merge pull request #5137 from asgerf/js/redux-less

Approved by erik-krogh

Author: codeql-ci

javascript/change-notes/2021-04-08-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

@@ -105,6 +105,7 @@ import semmle.javascript.frameworks.PropertyProjection
 import semmle.javascript.frameworks.Puppeteer
 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/BasicBlocks.qll

@@ -307,34 +307,30 @@ class ReachableBasicBlock extends BasicBlock {
   /**
    * Holds if this basic block strictly dominates `bb`.
    */
-  cached
+  pragma[inline]
   predicate strictlyDominates(ReachableBasicBlock bb) { bbIDominates+(this, bb) }
 
   /**
    * Holds if this basic block dominates `bb`.
    *
    * 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) }
 
   /**
    * Holds if this basic block post-dominates `bb`.
    *
    * 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/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/Configuration.qll

@@ -72,7 +72,7 @@ private import javascript
 private import internal.FlowSteps
 private import internal.AccessPaths
 private import internal.CallGraphs
-private import internal.Unit
+private import semmle.javascript.Unit
 private import semmle.javascript.internal.CachedStages
 
 /**

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

@@ -1683,4 +1683,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

@@ -52,6 +52,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

@@ -15,7 +15,7 @@
 
 import javascript
 private import semmle.javascript.dataflow.internal.FlowSteps as FlowSteps
-private import semmle.javascript.dataflow.internal.Unit
+private import semmle.javascript.Unit
 private import semmle.javascript.dataflow.InferredTypes
 private import semmle.javascript.internal.CachedStages
 

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

@@ -9,7 +9,7 @@
 private import javascript
 private import internal.FlowSteps
 private import internal.StepSummary
-private import internal.Unit
+private import semmle.javascript.Unit
 private import semmle.javascript.internal.CachedStages
 
 private newtype TTypeTracker = MkTypeTracker(Boolean hasCall, OptionalPropertyName prop)