Merge pull request #11398 from erik-krogh/splat-stuff

Rb: add some more flow through splat parameters

Author: erik-krogh

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

@@ -497,6 +497,7 @@ private module Cached {
       FlowSummaryImplSpecific::ParsePositions::isParsedKeywordParameterPosition(_, name)
     } or
     THashSplatArgumentPosition() or
+    TSplatAllArgumentPosition() or
     TAnyArgumentPosition() or
     TAnyKeywordArgumentPosition()
 
@@ -518,6 +519,7 @@ private module Cached {
       FlowSummaryImplSpecific::ParsePositions::isParsedKeywordArgumentPosition(_, name)
     } or
     THashSplatParameterPosition() or
+    TSplatAllParameterPosition() or
     TAnyParameterPosition() or
     TAnyKeywordParameterPosition()
 }
@@ -1149,6 +1151,8 @@ class ParameterPosition extends TParameterPosition {
   /** Holds if this position represents a hash-splat parameter. */
   predicate isHashSplat() { this = THashSplatParameterPosition() }
 
+  predicate isSplatAll() { this = TSplatAllParameterPosition() }
+
   /**
    * Holds if this position represents any parameter, except `self` parameters. This
    * includes both positional, named, and block parameters.
@@ -1172,6 +1176,8 @@ class ParameterPosition extends TParameterPosition {
     or
     this.isHashSplat() and result = "**"
     or
+    this.isSplatAll() and result = "*"
+    or
     this.isAny() and result = "any"
     or
     this.isAnyNamed() and result = "any-named"
@@ -1207,6 +1213,8 @@ class ArgumentPosition extends TArgumentPosition {
    */
   predicate isHashSplat() { this = THashSplatArgumentPosition() }
 
+  predicate isSplatAll() { this = TSplatAllArgumentPosition() }
+
   /** Gets a textual representation of this position. */
   string toString() {
     this.isSelf() and result = "self"
@@ -1222,6 +1230,8 @@ class ArgumentPosition extends TArgumentPosition {
     this.isAnyNamed() and result = "any-named"
     or
     this.isHashSplat() and result = "**"
+    or
+    this.isSplatAll() and result = "*"
   }
 }
 
@@ -1248,6 +1258,8 @@ predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) {
   or
   ppos.isHashSplat() and apos.isHashSplat()
   or
+  ppos.isSplatAll() and apos.isSplatAll()
+  or
   ppos.isAny() and argumentPositionIsNotSelf(apos)
   or
   apos.isAny() and parameterPositionIsNotSelf(ppos)

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

@@ -241,6 +241,10 @@ private class Argument extends CfgNodes::ExprCfgNode {
     this = call.getAnArgument() and
     this.getExpr() instanceof HashSplatExpr and
     arg.isHashSplat()
+    or
+    this = call.getArgument(0) and
+    this.getExpr() instanceof SplatExpr and
+    arg.isSplatAll()
   }
 
   /** Holds if this expression is the `i`th argument of `c`. */
@@ -276,7 +280,8 @@ private module Cached {
       p instanceof SimpleParameter or
       p instanceof OptionalParameter or
       p instanceof KeywordParameter or
-      p instanceof HashSplatParameter
+      p instanceof HashSplatParameter or
+      p instanceof SplatParameter
     } or
     TSelfParameterNode(MethodBase m) or
     TBlockParameterNode(MethodBase m) or
@@ -616,6 +621,9 @@ private module ParameterNodes {
         or
         parameter = callable.getAParameter().(HashSplatParameter) and
         pos.isHashSplat()
+        or
+        parameter = callable.getParameter(0).(SplatParameter) and
+        pos.isSplatAll()
       )
     }
 

ruby/ql/test/library-tests/dataflow/params/params-flow.expected

@@ -40,6 +40,11 @@ edges
 | params_flow.rb:49:13:49:14 | p1 :  | params_flow.rb:50:10:50:11 | p1 |
 | params_flow.rb:54:9:54:17 | call to taint :  | params_flow.rb:49:13:49:14 | p1 :  |
 | params_flow.rb:57:9:57:17 | call to taint :  | params_flow.rb:49:13:49:14 | p1 :  |
+| params_flow.rb:62:8:62:16 | call to taint :  | params_flow.rb:66:13:66:16 | args :  |
+| params_flow.rb:63:16:63:17 | *x [element 0] :  | params_flow.rb:64:10:64:10 | x [element 0] :  |
+| params_flow.rb:64:10:64:10 | x [element 0] :  | params_flow.rb:64:10:64:13 | ...[...] |
+| params_flow.rb:66:12:66:16 | * ... [element 0] :  | params_flow.rb:63:16:63:17 | *x [element 0] :  |
+| params_flow.rb:66:13:66:16 | args :  | params_flow.rb:66:12:66:16 | * ... [element 0] :  |
 nodes
 | params_flow.rb:9:16:9:17 | p1 :  | semmle.label | p1 :  |
 | params_flow.rb:9:20:9:21 | p2 :  | semmle.label | p2 :  |
@@ -89,6 +94,12 @@ nodes
 | params_flow.rb:50:10:50:11 | p1 | semmle.label | p1 |
 | params_flow.rb:54:9:54:17 | call to taint :  | semmle.label | call to taint :  |
 | params_flow.rb:57:9:57:17 | call to taint :  | semmle.label | call to taint :  |
+| params_flow.rb:62:8:62:16 | call to taint :  | semmle.label | call to taint :  |
+| params_flow.rb:63:16:63:17 | *x [element 0] :  | semmle.label | *x [element 0] :  |
+| params_flow.rb:64:10:64:10 | x [element 0] :  | semmle.label | x [element 0] :  |
+| params_flow.rb:64:10:64:13 | ...[...] | semmle.label | ...[...] |
+| params_flow.rb:66:12:66:16 | * ... [element 0] :  | semmle.label | * ... [element 0] :  |
+| params_flow.rb:66:13:66:16 | args :  | semmle.label | args :  |
 subpaths
 #select
 | params_flow.rb:10:10:10:11 | p1 | params_flow.rb:14:12:14:19 | call to taint :  | params_flow.rb:10:10:10:11 | p1 | $@ | params_flow.rb:14:12:14:19 | call to taint :  | call to taint :  |
@@ -111,3 +122,4 @@ subpaths
 | params_flow.rb:29:10:29:22 | ( ... ) | params_flow.rb:34:14:34:22 | call to taint :  | params_flow.rb:29:10:29:22 | ( ... ) | $@ | params_flow.rb:34:14:34:22 | call to taint :  | call to taint :  |
 | params_flow.rb:50:10:50:11 | p1 | params_flow.rb:54:9:54:17 | call to taint :  | params_flow.rb:50:10:50:11 | p1 | $@ | params_flow.rb:54:9:54:17 | call to taint :  | call to taint :  |
 | params_flow.rb:50:10:50:11 | p1 | params_flow.rb:57:9:57:17 | call to taint :  | params_flow.rb:50:10:50:11 | p1 | $@ | params_flow.rb:57:9:57:17 | call to taint :  | call to taint :  |
+| params_flow.rb:64:10:64:13 | ...[...] | params_flow.rb:62:8:62:16 | call to taint :  | params_flow.rb:64:10:64:13 | ...[...] | $@ | params_flow.rb:62:8:62:16 | call to taint :  | call to taint :  |

ruby/ql/test/library-tests/dataflow/params/params_flow.rb

@@ -57,4 +57,10 @@ def posargs(p1, *posargs)
 posargs(taint(23), *args)
 
 args = [taint(24), taint(25)]
-posargs(*args)
+posargs(*args)
+
+args = taint(26)
+def splatstuff(*x)
+    sink x[0] # $ hasValueFlow=26
+end
+splatstuff(*args)