More precise flow into splat parameters

We now precisely track flow from positional arguments to splat
parameters, provided that splat arguments are not used and there are no
positional parameters after the splat parameter. For example, in this
case:

    def f(x, y, *z); end

    f(a, b, c, d)

we get flow from `c` to `z[0]` and `d` to `z[1]`.

We get false flow if there are positional parameters after the splat
parameter. For example in this case:

    def g(x, y, *z, w); end

    g(a, b, c, d)

we get flow from `d` to `z[0]` instead of `w`.

We also track flow in this case

    def f(a, *b)
      sink b[0]
    end

    f(1, *[taint, 2])

Author: hmac

ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowDispatch.qll

@@ -440,6 +440,7 @@ private module Cached {
     THashSplatArgumentPosition() or
     TSplatAllArgumentPosition() or
     TSplatArgumentPosition(int pos) { exists(Call c | c.getArgument(pos) instanceof SplatExpr) } or
+    TSynthSplatArgumentPosition() or
     TAnyArgumentPosition() or
     TAnyKeywordArgumentPosition()
 
@@ -473,6 +474,7 @@ private module Cached {
       exists(Parameter p | p.getPosition() = pos and p instanceof SplatParameter)
     } or
     TSynthSplatParameterPosition() or
+    TSynthArgSplatParameterPosition() or
     TAnyParameterPosition() or
     TAnyKeywordParameterPosition()
 }
@@ -1295,6 +1297,9 @@ class ParameterPosition extends TParameterPosition {
 
   predicate isSynthSplat() { this = TSynthSplatParameterPosition() }
 
+  // A fake position to indicate that this parameter node holds content from a synth arg splat node
+  predicate isSynthArgSplat() { this = TSynthArgSplatParameterPosition() }
+
   predicate isSplatAll() { this = TSplatAllParameterPosition() }
 
   predicate isSplat(int n) { this = TSplatParameterPosition(n) }
@@ -1332,6 +1337,8 @@ class ParameterPosition extends TParameterPosition {
     or
     this.isSynthSplat() and result = "synthetic *"
     or
+    this.isSynthArgSplat() and result = "synthetic * (from *args)"
+    or
     exists(int pos | this.isSplat(pos) and result = "* (position " + pos + ")")
   }
 }
@@ -1369,6 +1376,8 @@ class ArgumentPosition extends TArgumentPosition {
 
   predicate isSplat(int n) { this = TSplatArgumentPosition(n) }
 
+  predicate isSynthSplat() { this = TSynthSplatArgumentPosition() }
+
   /** Gets a textual representation of this position. */
   string toString() {
     this.isSelf() and result = "self"
@@ -1387,6 +1396,8 @@ class ArgumentPosition extends TArgumentPosition {
     or
     this.isSplatAll() and result = "*"
     or
+    this.isSynthSplat() and result = "synthetic *"
+    or
     exists(int pos | this.isSplat(pos) and result = "* (position " + pos + ")")
   }
 }
@@ -1418,6 +1429,8 @@ predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) {
   or
   ppos.isSplatAll() and apos.isSplatAll()
   or
+  ppos.isSplatAll() and apos.isSynthSplat()
+  or
   ppos.isSynthSplat() and apos.isSplatAll()
   or
   // Exact splat match
@@ -1430,6 +1443,13 @@ predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) {
   ppos.isAnyNamed() and apos.isKeyword(_)
   or
   apos.isAnyNamed() and ppos.isKeyword(_)
+  or
+  ppos.isSynthSplat() and apos.isSplatAll()
+  or
+  // Exact splat match
+  exists(int n | apos.isSplat(n) and ppos.isSplat(n))
+  or
+  apos.isSynthSplat() and ppos.isSynthArgSplat()
 }
 
 /**

ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll

@@ -312,6 +312,15 @@ private module Cached {
       exists(c.asCallable()) and // exclude library callables
       isParameterNode(_, c, any(ParameterPosition p | p.isPositional(_)))
     } or
+    TSynthSplatArgParameterNode(DataFlowCallable c) {
+      exists(c.asCallable()) and // exclude library callables
+      isParameterNode(_, c, any(ParameterPosition p | p.isSplat(_)))
+    } or
+    TSynthSplatParameterElementNode(DataFlowCallable c, int n) {
+      exists(c.asCallable()) and // exclude library callables
+      exists(ArgumentPosition p | p.isPositional(n)) and
+      isParameterNode(_, c, any(ParameterPosition p | p.isSplat(_)))
+    } or
     TExprPostUpdateNode(CfgNodes::ExprCfgNode n) {
       // filter out nodes that clearly don't need post-update nodes
       isNonConstantExpr(n) and
@@ -326,11 +335,17 @@ private module Cached {
       exists(Argument arg | arg.isArgumentOf(c, any(ArgumentPosition pos | pos.isKeyword(_))))
       or
       c.getAnArgument() instanceof CfgNodes::ExprNodes::PairCfgNode
+    } or
+    TSynthSplatArgumentNode(CfgNodes::ExprNodes::CallCfgNode c) {
+      not exists(Argument arg, ArgumentPosition pos | pos.isSplat(_) or pos.isSplatAll() |
+        arg.isArgumentOf(c, pos)
+      )
     }
 
   class TSourceParameterNode =
     TNormalParameterNode or TBlockParameterNode or TSelfParameterNode or
-        TSynthHashSplatParameterNode or TSynthSplatParameterNode;
+        TSynthHashSplatParameterNode or TSynthSplatParameterNode or TSynthSplatArgParameterNode or
+        TSynthSplatParameterElementNode;
 
   cached
   Location getLocation(NodeImpl n) { result = n.getLocationImpl() }
@@ -528,6 +543,12 @@ predicate nodeIsHidden(Node n) {
   n instanceof SynthHashSplatArgumentNode
   or
   n instanceof SynthSplatParameterNode
+  or
+  n instanceof SynthSplatArgumentNode
+  or
+  n instanceof SynthSplatArgParameterNode
+  or
+  n instanceof SynthSplatParameterElementNode
 }
 
 /** An SSA definition, viewed as a node in a data flow graph. */
@@ -835,6 +856,50 @@ private module ParameterNodes {
     final override string toStringImpl() { result = "synthetic *args" }
   }
 
+  /**
+   * A node that holds all positional arguments passed in a call to `c`.
+   * This is a mirror of the `SynthSplatArgumentNode` on the callable side.
+   * See `SynthSplatArgumentNode` for more information.
+   */
+  class SynthSplatArgParameterNode extends ParameterNodeImpl, TSynthSplatArgParameterNode {
+    private DataFlowCallable callable;
+
+    SynthSplatArgParameterNode() { this = TSynthSplatArgParameterNode(callable) }
+
+    final override Parameter getParameter() { none() }
+
+    final override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) {
+      c = callable and pos.isSynthArgSplat()
+    }
+
+    final override CfgScope getCfgScope() { result = callable.asCallable() }
+
+    final override DataFlowCallable getEnclosingCallable() { result = callable }
+
+    final override Location getLocationImpl() { result = callable.getLocation() }
+
+    final override string toStringImpl() { result = "synthetic *args" }
+  }
+
+  /**
+   * A node that holds the content of a specific positional argument.
+   * See `SynthSplatArgumentNode` for more information.
+   */
+  class SynthSplatParameterElementNode extends TSynthSplatParameterElementNode {
+    private DataFlowCallable callable;
+    private int pos;
+
+    SynthSplatParameterElementNode() { this = TSynthSplatParameterElementNode(callable, pos) }
+
+    int getPosition() { result = pos }
+
+    DataFlowCallable getCallable() { result = callable }
+
+    string toString() { result = "synthetic *args[" + pos + "]" }
+
+    Location getLocation() { result = callable.getLocation() }
+  }
+
   /** A parameter for a library callable with a flow summary. */
   class SummaryParameterNode extends ParameterNodeImpl, FlowSummaryNode {
     private ParameterPosition pos_;
@@ -980,6 +1045,57 @@ private module ArgumentNodes {
 
     override string toStringImpl() { result = "**" }
   }
+
+  /**
+   * A data-flow node that represents all arguments passed to the call.
+   * We use this to model data flow via splat parameters.
+   * Consider this example:
+   *
+   * ```rb
+   * def foo(x, y, *z)
+   * end
+   *
+   * foo(1, 2, 3, 4)
+   * ```
+   *
+   * 1. We want `3` to flow to `z[0]` and `4` to flow to `z[1]`. We model this by first storing all arguments
+   *   in a synthetic argument node `SynthSplatArgumentNode` (see `storeStepCommon`).
+   * 2. We match this to an analogous parameter node `SynthSplatArgParameterNode` on the callee side
+   *   (see `parameterMatch`).
+   * 3. For each content element stored in the `SynthSplatArgParameterNode`, we add a read step to a separate
+   *   `SynthSplatParameterElementNode`, which is parameterised by the element index (see `readStep`).
+   * 4. Finally, we add store steps from these `SynthSplatParameterElementNode`s to the real splat parameter node
+   *   (see `storeStep`).
+   *   We only add store steps for elements that will not flow to the earlier positional parameters.
+   *   In practice that means we ignore elements at index `<= N`, where `N` is the index of the splat parameter.
+   *   For the remaining elements we subtract `N` from their index and store them in the splat parameter.
+   */
+  class SynthSplatArgumentNode extends ArgumentNode, TSynthSplatArgumentNode {
+    CfgNodes::ExprNodes::CallCfgNode c;
+
+    SynthSplatArgumentNode() { this = TSynthSplatArgumentNode(c) }
+
+    override predicate argumentOf(DataFlowCall call, ArgumentPosition pos) {
+      this.sourceArgumentOf(call.asCall(), pos)
+    }
+
+    override predicate sourceArgumentOf(CfgNodes::ExprNodes::CallCfgNode call, ArgumentPosition pos) {
+      call = c and
+      pos.isSynthSplat()
+    }
+  }
+
+  private class SynthSplatArgumentNodeImpl extends NodeImpl, TSynthSplatArgumentNode {
+    CfgNodes::ExprNodes::CallCfgNode c;
+
+    SynthSplatArgumentNodeImpl() { this = TSynthSplatArgumentNode(c) }
+
+    override CfgScope getCfgScope() { result = c.getExpr().getCfgScope() }
+
+    override Location getLocationImpl() { result = c.getLocation() }
+
+    override string toStringImpl() { result = "*" }
+  }
 }
 
 import ArgumentNodes
@@ -1215,6 +1331,22 @@ predicate storeStepCommon(Node node1, ContentSet c, Node node2) {
       c.isSingleton(TKnownElementContent(cv))
     )
   )
+  or
+  exists(CfgNodes::ExprNodes::CallCfgNode call, ArgumentPosition pos |
+    node2 = TSynthSplatArgumentNode(call) and
+    node1.asExpr().(Argument).isArgumentOf(call, pos)
+  |
+    exists(int n | pos.isPositional(n) and c = getPositionalContent(n))
+  )
+  or
+  // Store from SynthSplatParameterElementNode[n] into SplatParameterNode[m]
+  // where m = n - <position of SplatParameterNode>
+  exists(SynthSplatParameterElementNode elemNode, NormalParameterNode splatNode, int splatPos |
+    elemNode = node1 and splatNode = node2
+  |
+    splatNode.isParameterOf(elemNode.getCallable(), any(ParameterPosition p | p.isSplat(splatPos))) and
+    c = getPositionalContent(elemNode.getPosition() - splatPos)
+  )
 }
 
 /**
@@ -1284,6 +1416,15 @@ predicate readStep(Node node1, ContentSet c, Node node2) {
   or
   FlowSummaryImpl::Private::Steps::summaryReadStep(node1.(FlowSummaryNode).getSummaryNode(), c,
     node2.(FlowSummaryNode).getSummaryNode())
+  or
+  // Read from SynthSplatArgParameterNode into SynthSplatParameterElementNode
+  exists(SynthSplatArgParameterNode fromNode, SynthSplatParameterElementNode toNode, int pos |
+    node1 = fromNode and node2 = toNode
+  |
+    fromNode.isParameterOf(toNode.getCallable(), _) and
+    c = getPositionalContent(pos) and
+    toNode.getPosition() = pos
+  )
 }
 
 /**