Skip to content

Commit 7ebd511

Browse files
hmachmac
committed
Ruby: Handle more splat arg flow
Allow flow from a splat argument to a positional parameter in cases where there are positional arguments left of the splat. For example: def foo(x, y, z); end foo(1, *[2, 3])
1 parent 111227e commit 7ebd511

File tree

6 files changed

+133
-40
lines changed

6 files changed

+133
-40
lines changed

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,11 @@ private module Cached {
472472
TSplatParameterPosition(int pos) {
473473
exists(Parameter p | p.getPosition() = pos and p instanceof SplatParameter)
474474
} or
475-
TSynthSplatParameterPosition() or
475+
TSynthSplatParameterPosition(int pos) {
476+
// `pos` is the position of the splat _argument_ that is matched to the
477+
// `SynthSplatParameterNode` with this position.
478+
exists(ArgumentPosition a | a.isSplat(pos))
479+
} or
476480
TSynthArgSplatParameterPosition() or
477481
TAnyParameterPosition() or
478482
TAnyKeywordParameterPosition()
@@ -1301,7 +1305,7 @@ class ParameterPosition extends TParameterPosition {
13011305

13021306
predicate isSynthHashSplat() { this = TSynthHashSplatParameterPosition() }
13031307

1304-
predicate isSynthSplat() { this = TSynthSplatParameterPosition() }
1308+
predicate isSynthSplat(int n) { this = TSynthSplatParameterPosition(n) }
13051309

13061310
// A fake position to indicate that this parameter node holds content from a synth arg splat node
13071311
predicate isSynthArgSplat() { this = TSynthArgSplatParameterPosition() }
@@ -1339,7 +1343,7 @@ class ParameterPosition extends TParameterPosition {
13391343
or
13401344
this.isAnyNamed() and result = "any-named"
13411345
or
1342-
this.isSynthSplat() and result = "synthetic *"
1346+
exists(int pos | this.isSynthSplat(pos) and result = "synthetic * (position " + pos + ")")
13431347
or
13441348
this.isSynthArgSplat() and result = "synthetic * (from *args)"
13451349
or
@@ -1442,7 +1446,7 @@ predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) {
14421446
or
14431447
ppos.isSplat(0) and apos.isSynthSplat()
14441448
or
1445-
ppos.isSynthSplat() and apos.isSplat(0)
1449+
exists(int n | ppos.isSynthSplat(n) and apos.isSplat(n))
14461450
or
14471451
apos.isSynthSplat() and ppos.isSynthArgSplat()
14481452
or

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

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -448,9 +448,11 @@ private module Cached {
448448
TSynthHashSplatParameterNode(DataFlowCallable c) {
449449
isParameterNode(_, c, any(ParameterPosition p | p.isKeyword(_)))
450450
} or
451-
TSynthSplatParameterNode(DataFlowCallable c) {
451+
TSynthSplatParameterNode(DataFlowCallable c, int n) {
452452
exists(c.asCallable()) and // exclude library callables
453-
isParameterNode(_, c, any(ParameterPosition p | p.isPositional(_)))
453+
isParameterNode(_, c, any(ParameterPosition p | p.isPositional(_))) and
454+
// `n` is the position of the splat argument that is matched to this node
455+
exists(ArgumentPosition pos | pos.isSplat(n))
454456
} or
455457
TSynthSplatArgParameterNode(DataFlowCallable c) {
456458
exists(c.asCallable()) and // exclude library callables
@@ -1037,11 +1039,20 @@ private module ParameterNodes {
10371039
* ```rb
10381040
* foo(a, *[b])
10391041
* ```
1042+
*
1043+
* TODO: we do now support the above, but we don't support this case:
1044+
*
1045+
* ```rb
1046+
* foo(a, *[b], c)
1047+
* ```
1048+
*
1049+
* Update this documentation.
10401050
*/
10411051
class SynthSplatParameterNode extends ParameterNodeImpl, TSynthSplatParameterNode {
10421052
private DataFlowCallable callable;
1053+
private int n;
10431054

1044-
SynthSplatParameterNode() { this = TSynthSplatParameterNode(callable) }
1055+
SynthSplatParameterNode() { this = TSynthSplatParameterNode(callable, n) }
10451056

10461057
/**
10471058
* Gets a parameter which will contain the value given by `c`, assuming
@@ -1053,13 +1064,13 @@ private module ParameterNodes {
10531064
* end
10541065
* ```
10551066
*
1056-
* Then `getAParameter(element 0) = x` and `getAParameter(element 1) = y`.
1067+
* then `getAParameter(element 0) = x` and `getAParameter(element 1) = y`.
10571068
*/
10581069
ParameterNode getAParameter(ContentSet c) {
1059-
exists(int n |
1060-
isParameterNode(result, callable, (any(ParameterPosition p | p.isPositional(n)))) and
1070+
exists(int m |
1071+
isParameterNode(result, callable, (any(ParameterPosition p | p.isPositional(m)))) and
10611072
(
1062-
c = getPositionalContent(n)
1073+
c = getPositionalContent(m - n)
10631074
or
10641075
c.isSingleton(TUnknownElementContent())
10651076
)
@@ -1069,7 +1080,7 @@ private module ParameterNodes {
10691080
final override Parameter getParameter() { none() }
10701081

10711082
final override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) {
1072-
c = callable and pos.isSynthSplat()
1083+
c = callable and pos.isSynthSplat(n)
10731084
}
10741085

10751086
final override CfgScope getCfgScope() { result = callable.asCallable() }

‎ruby/ql/test/library-tests/dataflow/local/TaintStep.expected

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2796,7 +2796,6 @@
27962796
| UseUseExplosion.rb:21:3675:21:3680 | call to use | UseUseExplosion.rb:21:3670:21:3680 | else ... |
27972797
| UseUseExplosion.rb:21:3686:21:3696 | else ... | UseUseExplosion.rb:21:9:21:3700 | if ... |
27982798
| UseUseExplosion.rb:21:3691:21:3696 | call to use | UseUseExplosion.rb:21:3686:21:3696 | else ... |
2799-
| UseUseExplosion.rb:24:5:25:7 | synthetic *args | UseUseExplosion.rb:24:13:24:13 | i |
28002799
| UseUseExplosion.rb:24:5:25:7 | use | UseUseExplosion.rb:1:1:26:3 | C |
28012800
| file://:0:0:0:0 | [summary param] position 0 in & | file://:0:0:0:0 | [summary] read: Argument[0].Element[any] in & |
28022801
| file://:0:0:0:0 | [summary param] position 0 in + | file://:0:0:0:0 | [summary] read: Argument[0].Element[any] in + |
@@ -2841,7 +2840,6 @@
28412840
| file://:0:0:0:0 | [summary] read: Argument[0].Element[any] in Hash[] | file://:0:0:0:0 | [summary] read: Argument[0].Element[any].Element[1] in Hash[] |
28422841
| local_dataflow.rb:1:1:7:3 | self (foo) | local_dataflow.rb:3:8:3:10 | self |
28432842
| local_dataflow.rb:1:1:7:3 | self in foo | local_dataflow.rb:1:1:7:3 | self (foo) |
2844-
| local_dataflow.rb:1:1:7:3 | synthetic *args | local_dataflow.rb:1:9:1:9 | a |
28452843
| local_dataflow.rb:1:1:150:3 | <uninitialized> | local_dataflow.rb:10:9:10:9 | x |
28462844
| local_dataflow.rb:1:1:150:3 | self (local_dataflow.rb) | local_dataflow.rb:49:1:53:3 | self |
28472845
| local_dataflow.rb:1:9:1:9 | a | local_dataflow.rb:1:9:1:9 | a |
@@ -2879,7 +2877,6 @@
28792877
| local_dataflow.rb:10:5:13:3 | __synth__0__1 | local_dataflow.rb:10:5:13:3 | __synth__0__1 |
28802878
| local_dataflow.rb:10:5:13:3 | __synth__0__1 | local_dataflow.rb:10:9:10:9 | x |
28812879
| local_dataflow.rb:10:5:13:3 | call to each | local_dataflow.rb:10:5:13:3 | ... |
2882-
| local_dataflow.rb:10:5:13:3 | synthetic *args | local_dataflow.rb:10:5:13:3 | __synth__0__1 |
28832880
| local_dataflow.rb:10:9:10:9 | ... = ... | local_dataflow.rb:10:9:10:9 | if ... |
28842881
| local_dataflow.rb:10:9:10:9 | defined? ... | local_dataflow.rb:10:9:10:9 | [false] ! ... |
28852882
| local_dataflow.rb:10:9:10:9 | defined? ... | local_dataflow.rb:10:9:10:9 | [true] ! ... |
@@ -2898,7 +2895,6 @@
28982895
| local_dataflow.rb:15:1:17:3 | __synth__0__1 | local_dataflow.rb:15:1:17:3 | __synth__0__1 |
28992896
| local_dataflow.rb:15:1:17:3 | __synth__0__1 | local_dataflow.rb:15:5:15:5 | x |
29002897
| local_dataflow.rb:15:1:17:3 | call to each | local_dataflow.rb:15:1:17:3 | ... |
2901-
| local_dataflow.rb:15:1:17:3 | synthetic *args | local_dataflow.rb:15:1:17:3 | __synth__0__1 |
29022898
| local_dataflow.rb:15:5:15:5 | ... = ... | local_dataflow.rb:15:5:15:5 | if ... |
29032899
| local_dataflow.rb:15:5:15:5 | defined? ... | local_dataflow.rb:15:5:15:5 | [false] ! ... |
29042900
| local_dataflow.rb:15:5:15:5 | defined? ... | local_dataflow.rb:15:5:15:5 | [true] ! ... |
@@ -2914,7 +2910,6 @@
29142910
| local_dataflow.rb:19:1:21:3 | __synth__0__1 | local_dataflow.rb:19:1:21:3 | __synth__0__1 |
29152911
| local_dataflow.rb:19:1:21:3 | __synth__0__1 | local_dataflow.rb:19:5:19:5 | x |
29162912
| local_dataflow.rb:19:1:21:3 | call to each | local_dataflow.rb:19:1:21:3 | ... |
2917-
| local_dataflow.rb:19:1:21:3 | synthetic *args | local_dataflow.rb:19:1:21:3 | __synth__0__1 |
29182913
| local_dataflow.rb:19:5:19:5 | ... = ... | local_dataflow.rb:19:5:19:5 | if ... |
29192914
| local_dataflow.rb:19:5:19:5 | defined? ... | local_dataflow.rb:19:5:19:5 | [false] ! ... |
29202915
| local_dataflow.rb:19:5:19:5 | defined? ... | local_dataflow.rb:19:5:19:5 | [true] ! ... |
@@ -2933,13 +2928,11 @@
29332928
| local_dataflow.rb:30:14:30:20 | "class" | local_dataflow.rb:30:5:30:24 | C |
29342929
| local_dataflow.rb:32:5:32:25 | bar | local_dataflow.rb:32:1:32:1 | x |
29352930
| local_dataflow.rb:32:5:32:25 | bar | local_dataflow.rb:32:1:32:25 | ... = ... |
2936-
| local_dataflow.rb:34:1:39:3 | synthetic *args | local_dataflow.rb:34:7:34:7 | x |
29372931
| local_dataflow.rb:34:7:34:7 | x | local_dataflow.rb:34:7:34:7 | x |
29382932
| local_dataflow.rb:34:7:34:7 | x | local_dataflow.rb:35:6:35:6 | x |
29392933
| local_dataflow.rb:35:6:35:6 | x | local_dataflow.rb:35:6:35:11 | ... == ... |
29402934
| local_dataflow.rb:35:11:35:11 | 4 | local_dataflow.rb:35:6:35:11 | ... == ... |
29412935
| local_dataflow.rb:36:13:36:13 | 7 | local_dataflow.rb:36:6:36:13 | return |
2942-
| local_dataflow.rb:41:1:47:3 | synthetic *args | local_dataflow.rb:41:7:41:7 | x |
29432936
| local_dataflow.rb:41:7:41:7 | x | local_dataflow.rb:41:7:41:7 | x |
29442937
| local_dataflow.rb:41:7:41:7 | x | local_dataflow.rb:42:6:42:6 | x |
29452938
| local_dataflow.rb:42:6:42:6 | x | local_dataflow.rb:42:6:42:11 | ... == ... |
@@ -2958,10 +2951,8 @@
29582951
| local_dataflow.rb:51:20:51:20 | x | local_dataflow.rb:51:20:51:24 | ... < ... |
29592952
| local_dataflow.rb:51:24:51:24 | 9 | local_dataflow.rb:51:20:51:24 | ... < ... |
29602953
| local_dataflow.rb:55:5:55:13 | Array | local_dataflow.rb:55:5:55:13 | call to [] |
2961-
| local_dataflow.rb:57:1:58:3 | synthetic *args | local_dataflow.rb:57:9:57:9 | x |
29622954
| local_dataflow.rb:60:1:90:3 | self (test_case) | local_dataflow.rb:78:12:78:20 | self |
29632955
| local_dataflow.rb:60:1:90:3 | self in test_case | local_dataflow.rb:60:1:90:3 | self (test_case) |
2964-
| local_dataflow.rb:60:1:90:3 | synthetic *args | local_dataflow.rb:60:15:60:15 | x |
29652956
| local_dataflow.rb:60:15:60:15 | x | local_dataflow.rb:60:15:60:15 | x |
29662957
| local_dataflow.rb:60:15:60:15 | x | local_dataflow.rb:61:12:61:12 | x |
29672958
| local_dataflow.rb:61:7:68:5 | SSA phi read(x) | local_dataflow.rb:69:12:69:12 | x |
@@ -3134,7 +3125,6 @@
31343125
| local_dataflow.rb:118:3:118:11 | call to source | local_dataflow.rb:118:3:118:31 | call to tap |
31353126
| local_dataflow.rb:118:3:118:11 | self | local_dataflow.rb:119:3:119:31 | self |
31363127
| local_dataflow.rb:118:17:118:31 | <captured entry> self | local_dataflow.rb:118:23:118:29 | self |
3137-
| local_dataflow.rb:118:17:118:31 | synthetic *args | local_dataflow.rb:118:20:118:20 | x |
31383128
| local_dataflow.rb:118:20:118:20 | x | local_dataflow.rb:118:20:118:20 | x |
31393129
| local_dataflow.rb:118:20:118:20 | x | local_dataflow.rb:118:28:118:28 | x |
31403130
| local_dataflow.rb:119:3:119:31 | [post] self | local_dataflow.rb:119:8:119:16 | self |
@@ -3149,10 +3139,8 @@
31493139
| local_dataflow.rb:123:8:123:20 | call to dup | local_dataflow.rb:123:8:123:45 | call to tap |
31503140
| local_dataflow.rb:123:8:123:45 | call to tap | local_dataflow.rb:123:8:123:49 | call to dup |
31513141
| local_dataflow.rb:123:26:123:45 | <captured entry> self | local_dataflow.rb:123:32:123:43 | self |
3152-
| local_dataflow.rb:123:26:123:45 | synthetic *args | local_dataflow.rb:123:29:123:29 | x |
31533142
| local_dataflow.rb:126:1:128:3 | self (use) | local_dataflow.rb:127:3:127:8 | self |
31543143
| local_dataflow.rb:126:1:128:3 | self in use | local_dataflow.rb:126:1:128:3 | self (use) |
3155-
| local_dataflow.rb:126:1:128:3 | synthetic *args | local_dataflow.rb:126:9:126:9 | x |
31563144
| local_dataflow.rb:130:1:150:3 | self (use_use_madness) | local_dataflow.rb:132:6:132:11 | self |
31573145
| local_dataflow.rb:130:1:150:3 | self in use_use_madness | local_dataflow.rb:130:1:150:3 | self (use_use_madness) |
31583146
| local_dataflow.rb:131:3:131:3 | x | local_dataflow.rb:132:10:132:10 | x |

0 commit comments

Comments
 (0)