Skip to content

Commit fb4b774

Browse files
hmachmac
authored
Merge pull request github#13967 from hmac/remove-splat-all
Ruby: Remove isSplatAll
2 parents 1f1d48f + 414ae76 commit fb4b774

File tree

4 files changed

+47
-30
lines changed

4 files changed

+47
-30
lines changed

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

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -438,7 +438,6 @@ private module Cached {
438438
FlowSummaryImplSpecific::ParsePositions::isParsedKeywordParameterPosition(_, name)
439439
} or
440440
THashSplatArgumentPosition() or
441-
TSplatAllArgumentPosition() or
442441
TSplatArgumentPosition(int pos) { exists(Call c | c.getArgument(pos) instanceof SplatExpr) } or
443442
TSynthSplatArgumentPosition() or
444443
TAnyArgumentPosition() or
@@ -469,7 +468,6 @@ private module Cached {
469468
// position for multiple parameter nodes in the same callable, we introduce this
470469
// synthetic parameter position.
471470
TSynthHashSplatParameterPosition() or
472-
TSplatAllParameterPosition() or
473471
TSplatParameterPosition(int pos) {
474472
exists(Parameter p | p.getPosition() = pos and p instanceof SplatParameter)
475473
} or
@@ -1300,8 +1298,6 @@ class ParameterPosition extends TParameterPosition {
13001298
// A fake position to indicate that this parameter node holds content from a synth arg splat node
13011299
predicate isSynthArgSplat() { this = TSynthArgSplatParameterPosition() }
13021300

1303-
predicate isSplatAll() { this = TSplatAllParameterPosition() }
1304-
13051301
predicate isSplat(int n) { this = TSplatParameterPosition(n) }
13061302

13071303
/**
@@ -1329,8 +1325,6 @@ class ParameterPosition extends TParameterPosition {
13291325
or
13301326
this.isSynthHashSplat() and result = "synthetic **"
13311327
or
1332-
this.isSplatAll() and result = "*"
1333-
or
13341328
this.isAny() and result = "any"
13351329
or
13361330
this.isAnyNamed() and result = "any-named"
@@ -1372,8 +1366,6 @@ class ArgumentPosition extends TArgumentPosition {
13721366
*/
13731367
predicate isHashSplat() { this = THashSplatArgumentPosition() }
13741368

1375-
predicate isSplatAll() { this = TSplatAllArgumentPosition() }
1376-
13771369
predicate isSplat(int n) { this = TSplatArgumentPosition(n) }
13781370

13791371
predicate isSynthSplat() { this = TSynthSplatArgumentPosition() }
@@ -1394,8 +1386,6 @@ class ArgumentPosition extends TArgumentPosition {
13941386
or
13951387
this.isHashSplat() and result = "**"
13961388
or
1397-
this.isSplatAll() and result = "*"
1398-
or
13991389
this.isSynthSplat() and result = "synthetic *"
14001390
or
14011391
exists(int pos | this.isSplat(pos) and result = "* (position " + pos + ")")
@@ -1427,11 +1417,9 @@ predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) {
14271417
or
14281418
ppos.isSynthHashSplat() and apos.isHashSplat()
14291419
or
1430-
ppos.isSplatAll() and apos.isSplatAll()
1431-
or
1432-
ppos.isSplatAll() and apos.isSynthSplat()
1420+
ppos.isSplat(0) and apos.isSynthSplat()
14331421
or
1434-
ppos.isSynthSplat() and apos.isSplatAll()
1422+
ppos.isSynthSplat() and apos.isSplat(0)
14351423
or
14361424
apos.isSynthSplat() and ppos.isSynthArgSplat()
14371425
or

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

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -245,12 +245,7 @@ private class Argument extends CfgNodes::ExprCfgNode {
245245
this.getExpr() instanceof HashSplatExpr and
246246
arg.isHashSplat()
247247
or
248-
this = call.getArgument(0) and
249-
not exists(call.getArgument(1)) and
250-
this.getExpr() instanceof SplatExpr and
251-
arg.isSplatAll()
252-
or
253-
exists(int pos | pos > 0 or exists(call.getArgument(pos + 1)) |
248+
exists(int pos |
254249
this = call.getArgument(pos) and
255250
this.getExpr() instanceof SplatExpr and
256251
arg.isSplat(pos)
@@ -370,9 +365,7 @@ private module Cached {
370365
} or
371366
TSynthSplatArgumentNode(CfgNodes::ExprNodes::CallCfgNode c) {
372367
exists(Argument arg, ArgumentPosition pos | pos.isPositional(_) | arg.isArgumentOf(c, pos)) and
373-
not exists(Argument arg, ArgumentPosition pos | pos.isSplat(_) or pos.isSplatAll() |
374-
arg.isArgumentOf(c, pos)
375-
)
368+
not exists(Argument arg, ArgumentPosition pos | pos.isSplat(_) | arg.isArgumentOf(c, pos))
376369
}
377370

378371
class TSourceParameterNode =
@@ -697,11 +690,7 @@ private module ParameterNodes {
697690
parameter = callable.getAParameter().(HashSplatParameter) and
698691
pos.isHashSplat()
699692
or
700-
parameter = callable.getParameter(0).(SplatParameter) and
701-
not exists(callable.getParameter(1)) and
702-
pos.isSplatAll()
703-
or
704-
exists(int n | n > 0 |
693+
exists(int n |
705694
parameter = callable.getParameter(n).(SplatParameter) and
706695
pos.isSplat(n) and
707696
// There are no positional parameters after the splat

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,19 @@ edges
107107
| params_flow.rb:118:12:118:13 | * ... [element] | params_flow.rb:9:16:9:17 | p1 |
108108
| params_flow.rb:118:12:118:13 | * ... [element] | params_flow.rb:9:20:9:21 | p2 |
109109
| params_flow.rb:118:13:118:13 | x [element] | params_flow.rb:118:12:118:13 | * ... [element] |
110+
| params_flow.rb:130:1:130:4 | args [element 0] | params_flow.rb:131:11:131:14 | args [element 0] |
111+
| params_flow.rb:130:1:130:4 | args [element 1] | params_flow.rb:131:11:131:14 | args [element 1] |
112+
| params_flow.rb:130:9:130:17 | call to taint | params_flow.rb:130:1:130:4 | args [element 0] |
113+
| params_flow.rb:130:20:130:28 | call to taint | params_flow.rb:130:1:130:4 | args [element 1] |
114+
| params_flow.rb:131:10:131:14 | * ... [element 0] | params_flow.rb:83:14:83:14 | t |
115+
| params_flow.rb:131:10:131:14 | * ... [element 1] | params_flow.rb:83:17:83:17 | u |
116+
| params_flow.rb:131:11:131:14 | args [element 0] | params_flow.rb:131:10:131:14 | * ... [element 0] |
117+
| params_flow.rb:131:11:131:14 | args [element 1] | params_flow.rb:131:10:131:14 | * ... [element 1] |
118+
| params_flow.rb:131:17:131:25 | call to taint | params_flow.rb:83:17:83:17 | u |
119+
| params_flow.rb:133:14:133:18 | *args [element 1] | params_flow.rb:134:10:134:13 | args [element 1] |
120+
| params_flow.rb:134:10:134:13 | args [element 1] | params_flow.rb:134:10:134:16 | ...[...] |
121+
| params_flow.rb:137:10:137:43 | * ... [element 1] | params_flow.rb:133:14:133:18 | *args [element 1] |
122+
| params_flow.rb:137:23:137:31 | call to taint | params_flow.rb:137:10:137:43 | * ... [element 1] |
110123
nodes
111124
| params_flow.rb:9:16:9:17 | p1 | semmle.label | p1 |
112125
| params_flow.rb:9:20:9:21 | p2 | semmle.label | p2 |
@@ -235,6 +248,20 @@ nodes
235248
| params_flow.rb:117:19:117:27 | call to taint | semmle.label | call to taint |
236249
| params_flow.rb:118:12:118:13 | * ... [element] | semmle.label | * ... [element] |
237250
| params_flow.rb:118:13:118:13 | x [element] | semmle.label | x [element] |
251+
| params_flow.rb:130:1:130:4 | args [element 0] | semmle.label | args [element 0] |
252+
| params_flow.rb:130:1:130:4 | args [element 1] | semmle.label | args [element 1] |
253+
| params_flow.rb:130:9:130:17 | call to taint | semmle.label | call to taint |
254+
| params_flow.rb:130:20:130:28 | call to taint | semmle.label | call to taint |
255+
| params_flow.rb:131:10:131:14 | * ... [element 0] | semmle.label | * ... [element 0] |
256+
| params_flow.rb:131:10:131:14 | * ... [element 1] | semmle.label | * ... [element 1] |
257+
| params_flow.rb:131:11:131:14 | args [element 0] | semmle.label | args [element 0] |
258+
| params_flow.rb:131:11:131:14 | args [element 1] | semmle.label | args [element 1] |
259+
| params_flow.rb:131:17:131:25 | call to taint | semmle.label | call to taint |
260+
| params_flow.rb:133:14:133:18 | *args [element 1] | semmle.label | *args [element 1] |
261+
| params_flow.rb:134:10:134:13 | args [element 1] | semmle.label | args [element 1] |
262+
| params_flow.rb:134:10:134:16 | ...[...] | semmle.label | ...[...] |
263+
| params_flow.rb:137:10:137:43 | * ... [element 1] | semmle.label | * ... [element 1] |
264+
| params_flow.rb:137:23:137:31 | call to taint | semmle.label | call to taint |
238265
subpaths
239266
#select
240267
| 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,11 +302,15 @@ subpaths
275302
| 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 |
276303
| 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 |
277304
| 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 |
305+
| 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 |
278306
| 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 |
307+
| 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 |
308+
| 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 |
279309
| 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 |
280310
| 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 |
281311
| 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 |
282312
| params_flow.rb:102:10:102:10 | b | params_flow.rb:106:37:106:45 | call to taint | params_flow.rb:102:10:102:10 | b | $@ | params_flow.rb:106:37:106:45 | call to taint | call to taint |
283313
| 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 |
284314
| 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 |
285315
| 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 |
316+
| 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 |

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,8 @@ def splatmid(x, y, *z, w, r)
8181
splatmid(taint(32), *args, taint(37))
8282

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

128128
destruct([taint(62), taint(63)], [taint(64), [0, taint(65)]])
129+
130+
args = [taint(66), taint(67)]
131+
pos_many(*args, taint(68), nil, nil, nil, nil)
132+
133+
def splatall(*args)
134+
sink args[1] # $ hasValueFlow=70
135+
end
136+
137+
splatall(*[taint(69), taint(70), taint(71)])

0 commit comments

Comments
 (0)