Merge pull request #13967 from hmac/remove-splat-all

Ruby: Remove isSplatAll
2026-04-26 09:15:12 +02:00 · 2023-08-23 09:40:06 +01:00
parent 1f1d48f768 414ae76ae1
commit fb4b774c0d
4 changed files with 47 additions and 30 deletions
--- a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowDispatch.qll
+++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowDispatch.qll
@@ -438,7 +438,6 @@ private module Cached {
      FlowSummaryImplSpecific::ParsePositions::isParsedKeywordParameterPosition(_, name)
    } or
    THashSplatArgumentPosition() or
-    TSplatAllArgumentPosition() or
    TSplatArgumentPosition(int pos) { exists(Call c | c.getArgument(pos) instanceof SplatExpr) } or
    TSynthSplatArgumentPosition() or
    TAnyArgumentPosition() or
@@ -469,7 +468,6 @@ private module Cached {
    // position for multiple parameter nodes in the same callable, we introduce this
    // synthetic parameter position.
    TSynthHashSplatParameterPosition() or
-    TSplatAllParameterPosition() or
    TSplatParameterPosition(int pos) {
      exists(Parameter p | p.getPosition() = pos and p instanceof SplatParameter)
    } or
@@ -1300,8 +1298,6 @@ class ParameterPosition extends TParameterPosition {
  // 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) }

  /**
@@ -1329,8 +1325,6 @@ class ParameterPosition extends TParameterPosition {
    or
    this.isSynthHashSplat() and result = "synthetic **"
    or
-    this.isSplatAll() and result = "*"
-    or
    this.isAny() and result = "any"
    or
    this.isAnyNamed() and result = "any-named"
@@ -1372,8 +1366,6 @@ class ArgumentPosition extends TArgumentPosition {
   */
  predicate isHashSplat() { this = THashSplatArgumentPosition() }

-  predicate isSplatAll() { this = TSplatAllArgumentPosition() }
-
  predicate isSplat(int n) { this = TSplatArgumentPosition(n) }

  predicate isSynthSplat() { this = TSynthSplatArgumentPosition() }
@@ -1394,8 +1386,6 @@ class ArgumentPosition extends TArgumentPosition {
    or
    this.isHashSplat() and result = "**"
    or
-    this.isSplatAll() and result = "*"
-    or
    this.isSynthSplat() and result = "synthetic *"
    or
    exists(int pos | this.isSplat(pos) and result = "* (position " + pos + ")")
@@ -1427,11 +1417,9 @@ predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) {
  or
  ppos.isSynthHashSplat() and apos.isHashSplat()
  or
-  ppos.isSplatAll() and apos.isSplatAll()
+  ppos.isSplat(0) and apos.isSynthSplat()
  or
-  ppos.isSplatAll() and apos.isSynthSplat()
-  or
-  ppos.isSynthSplat() and apos.isSplatAll()
+  ppos.isSynthSplat() and apos.isSplat(0)
  or
  apos.isSynthSplat() and ppos.isSynthArgSplat()
  or
--- a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll
+++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll
@@ -245,12 +245,7 @@ private class Argument extends CfgNodes::ExprCfgNode {
    this.getExpr() instanceof HashSplatExpr and
    arg.isHashSplat()
    or
-    this = call.getArgument(0) and
-    not exists(call.getArgument(1)) and
-    this.getExpr() instanceof SplatExpr and
-    arg.isSplatAll()
-    or
-    exists(int pos | pos > 0 or exists(call.getArgument(pos + 1)) |
+    exists(int pos |
      this = call.getArgument(pos) and
      this.getExpr() instanceof SplatExpr and
      arg.isSplat(pos)
@@ -370,9 +365,7 @@ private module Cached {
    } or
    TSynthSplatArgumentNode(CfgNodes::ExprNodes::CallCfgNode c) {
      exists(Argument arg, ArgumentPosition pos | pos.isPositional(_) | arg.isArgumentOf(c, pos)) and
-      not exists(Argument arg, ArgumentPosition pos | pos.isSplat(_) or pos.isSplatAll() |
-        arg.isArgumentOf(c, pos)
-      )
+      not exists(Argument arg, ArgumentPosition pos | pos.isSplat(_) | arg.isArgumentOf(c, pos))
    }

  class TSourceParameterNode =
@@ -697,11 +690,7 @@ private module ParameterNodes {
        parameter = callable.getAParameter().(HashSplatParameter) and
        pos.isHashSplat()
        or
-        parameter = callable.getParameter(0).(SplatParameter) and
-        not exists(callable.getParameter(1)) and
-        pos.isSplatAll()
-        or
-        exists(int n | n > 0 |
+        exists(int n |
          parameter = callable.getParameter(n).(SplatParameter) and
          pos.isSplat(n) and
          // There are no positional parameters after the splat
--- a/ruby/ql/test/library-tests/dataflow/params/params-flow.expected
+++ b/ruby/ql/test/library-tests/dataflow/params/params-flow.expected
@@ -107,6 +107,19 @@ edges
 | params_flow.rb:118:12:118:13 | * ... [element] | params_flow.rb:9:16:9:17 | p1 |
 | params_flow.rb:118:12:118:13 | * ... [element] | params_flow.rb:9:20:9:21 | p2 |
 | params_flow.rb:118:13:118:13 | x [element] | params_flow.rb:118:12:118:13 | * ... [element] |
+| params_flow.rb:130:1:130:4 | args [element 0] | params_flow.rb:131:11:131:14 | args [element 0] |
+| params_flow.rb:130:1:130:4 | args [element 1] | params_flow.rb:131:11:131:14 | args [element 1] |
+| params_flow.rb:130:9:130:17 | call to taint | params_flow.rb:130:1:130:4 | args [element 0] |
+| params_flow.rb:130:20:130:28 | call to taint | params_flow.rb:130:1:130:4 | args [element 1] |
+| params_flow.rb:131:10:131:14 | * ... [element 0] | params_flow.rb:83:14:83:14 | t |
+| params_flow.rb:131:10:131:14 | * ... [element 1] | params_flow.rb:83:17:83:17 | u |
+| params_flow.rb:131:11:131:14 | args [element 0] | params_flow.rb:131:10:131:14 | * ... [element 0] |
+| params_flow.rb:131:11:131:14 | args [element 1] | params_flow.rb:131:10:131:14 | * ... [element 1] |
+| params_flow.rb:131:17:131:25 | call to taint | params_flow.rb:83:17:83:17 | u |
+| params_flow.rb:133:14:133:18 | *args [element 1] | params_flow.rb:134:10:134:13 | args [element 1] |
+| params_flow.rb:134:10:134:13 | args [element 1] | params_flow.rb:134:10:134:16 | ...[...] |
+| params_flow.rb:137:10:137:43 | * ... [element 1] | params_flow.rb:133:14:133:18 | *args [element 1] |
+| params_flow.rb:137:23:137:31 | call to taint | params_flow.rb:137:10:137:43 | * ... [element 1] |
 nodes
 | params_flow.rb:9:16:9:17 | p1 | semmle.label | p1 |
 | params_flow.rb:9:20:9:21 | p2 | semmle.label | p2 |
@@ -235,6 +248,20 @@ nodes
 | params_flow.rb:117:19:117:27 | call to taint | semmle.label | call to taint |
 | params_flow.rb:118:12:118:13 | * ... [element] | semmle.label | * ... [element] |
 | params_flow.rb:118:13:118:13 | x [element] | semmle.label | x [element] |
+| params_flow.rb:130:1:130:4 | args [element 0] | semmle.label | args [element 0] |
+| params_flow.rb:130:1:130:4 | args [element 1] | semmle.label | args [element 1] |
+| params_flow.rb:130:9:130:17 | call to taint | semmle.label | call to taint |
+| params_flow.rb:130:20:130:28 | call to taint | semmle.label | call to taint |
+| params_flow.rb:131:10:131:14 | * ... [element 0] | semmle.label | * ... [element 0] |
+| params_flow.rb:131:10:131:14 | * ... [element 1] | semmle.label | * ... [element 1] |
+| params_flow.rb:131:11:131:14 | args [element 0] | semmle.label | args [element 0] |
+| params_flow.rb:131:11:131:14 | args [element 1] | semmle.label | args [element 1] |
+| params_flow.rb:131:17:131:25 | call to taint | semmle.label | call to taint |
+| params_flow.rb:133:14:133:18 | *args [element 1] | semmle.label | *args [element 1] |
+| params_flow.rb:134:10:134:13 | args [element 1] | semmle.label | args [element 1] |
+| params_flow.rb:134:10:134:16 | ...[...] | semmle.label | ...[...] |
+| params_flow.rb:137:10:137:43 | * ... [element 1] | semmle.label | * ... [element 1] |
+| params_flow.rb:137:23:137:31 | call to taint | semmle.label | call to taint |
 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 |
@@ -275,7 +302,10 @@ subpaths
 | params_flow.rb:75:10:75:10 | r | params_flow.rb:78:54:78:62 | call to taint | params_flow.rb:75:10:75:10 | r | $@ | params_flow.rb:78:54:78:62 | call to taint | call to taint |
 | params_flow.rb:75:10:75:10 | r | params_flow.rb:96:79:96:87 | call to taint | params_flow.rb:75:10:75:10 | r | $@ | params_flow.rb:96:79:96:87 | call to taint | call to taint |
 | params_flow.rb:84:10:84:10 | t | params_flow.rb:94:10:94:18 | call to taint | params_flow.rb:84:10:84:10 | t | $@ | params_flow.rb:94:10:94:18 | call to taint | call to taint |
+| params_flow.rb:84:10:84:10 | t | params_flow.rb:130:9:130:17 | call to taint | params_flow.rb:84:10:84:10 | t | $@ | params_flow.rb:130:9:130:17 | call to taint | call to taint |
 | params_flow.rb:85:10:85:10 | u | params_flow.rb:94:21:94:29 | call to taint | params_flow.rb:85:10:85:10 | u | $@ | params_flow.rb:94:21:94:29 | call to taint | call to taint |
+| params_flow.rb:85:10:85:10 | u | params_flow.rb:130:20:130:28 | call to taint | params_flow.rb:85:10:85:10 | u | $@ | params_flow.rb:130:20:130:28 | call to taint | call to taint |
+| params_flow.rb:85:10:85:10 | u | params_flow.rb:131:17:131:25 | call to taint | params_flow.rb:85:10:85:10 | u | $@ | params_flow.rb:131:17:131:25 | call to taint | call to taint |
 | params_flow.rb:87:10:87:10 | w | params_flow.rb:94:39:94:47 | call to taint | params_flow.rb:87:10:87:10 | w | $@ | params_flow.rb:94:39:94:47 | call to taint | call to taint |
 | params_flow.rb:99:10:99:10 | a | params_flow.rb:105:15:105:23 | call to taint | params_flow.rb:99:10:99:10 | a | $@ | params_flow.rb:105:15:105:23 | call to taint | call to taint |
 | params_flow.rb:99:10:99:10 | a | params_flow.rb:106:15:106:23 | call to taint | params_flow.rb:99:10:99:10 | a | $@ | params_flow.rb:106:15:106:23 | call to taint | call to taint |
@@ -283,3 +313,4 @@ subpaths
 | params_flow.rb:109:10:109:10 | a | params_flow.rb:114:33:114:41 | call to taint | params_flow.rb:109:10:109:10 | a | $@ | params_flow.rb:114:33:114:41 | call to taint | call to taint |
 | params_flow.rb:110:10:110:13 | ...[...] | params_flow.rb:114:44:114:52 | call to taint | params_flow.rb:110:10:110:13 | ...[...] | $@ | params_flow.rb:114:44:114:52 | call to taint | call to taint |
 | params_flow.rb:111:10:111:10 | c | params_flow.rb:114:58:114:66 | call to taint | params_flow.rb:111:10:111:10 | c | $@ | params_flow.rb:114:58:114:66 | call to taint | call to taint |
+| params_flow.rb:134:10:134:16 | ...[...] | params_flow.rb:137:23:137:31 | call to taint | params_flow.rb:134:10:134:16 | ...[...] | $@ | params_flow.rb:137:23:137:31 | call to taint | call to taint |
--- a/ruby/ql/test/library-tests/dataflow/params/params_flow.rb
+++ b/ruby/ql/test/library-tests/dataflow/params/params_flow.rb
@@ -81,8 +81,8 @@ args = [taint(33), taint(34), taint(35), taint(36)]
 splatmid(taint(32), *args, taint(37))

 def pos_many(t, u, v, w, x, y, z)
-    sink t # $ hasValueFlow=38
-    sink u # $ hasValueFlow=39
+    sink t # $ hasValueFlow=38 $ hasValueFlow=66
+    sink u # $ hasValueFlow=39 $ hasValueFlow=67 $ SPURIOUS: hasValueFlow=68
    sink v # $ MISSING: hasValueFlow=40
    sink w # $ MISSING: hasValueFlow=41 $ SPURIOUS: hasValueFlow=44
    sink x # $ MISSING: hasValueFlow=42
@@ -126,3 +126,12 @@ def destruct((a,b), (c,(d,e)))
 end

 destruct([taint(62), taint(63)], [taint(64), [0, taint(65)]])
+
+args = [taint(66), taint(67)]
+pos_many(*args, taint(68), nil, nil, nil, nil)
+
+def splatall(*args)
+    sink args[1] # $ hasValueFlow=70
+end
+
+splatall(*[taint(69), taint(70), taint(71)])