Skip to content

Commit bd09c61

Browse files
hvitvedhvitved
authored
Merge pull request github#8786 from hvitved/ruby/dataflow/argument-tokens
Ruby: Implement `Argument[any]` and `Argument[n..]`
2 parents 0ec5aa6 + ea229d3 commit bd09c61

File tree

12 files changed

+103
-46
lines changed

12 files changed

+103
-46
lines changed

‎csharp/ql/lib/semmle/code/csharp/dataflow/internal/AccessPathSyntax.qll

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,7 @@ module AccessPath {
4242
* Parses a lower-bounded interval `n..` and gets the lower bound.
4343
*/
4444
bindingset[arg]
45-
private int parseLowerBound(string arg) {
46-
result = arg.regexpCapture("(-?\\d+)\\.\\.", 1).toInt()
47-
}
45+
int parseLowerBound(string arg) { result = arg.regexpCapture("(-?\\d+)\\.\\.", 1).toInt() }
4846

4947
/**
5048
* Parses an integer constant or interval (bounded or unbounded) that explicitly

‎java/ql/lib/semmle/code/java/dataflow/internal/AccessPathSyntax.qll

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,7 @@ module AccessPath {
4242
* Parses a lower-bounded interval `n..` and gets the lower bound.
4343
*/
4444
bindingset[arg]
45-
private int parseLowerBound(string arg) {
46-
result = arg.regexpCapture("(-?\\d+)\\.\\.", 1).toInt()
47-
}
45+
int parseLowerBound(string arg) { result = arg.regexpCapture("(-?\\d+)\\.\\.", 1).toInt() }
4846

4947
/**
5048
* Parses an integer constant or interval (bounded or unbounded) that explicitly

‎javascript/ql/lib/semmle/javascript/frameworks/data/internal/AccessPathSyntax.qll

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,7 @@ module AccessPath {
4242
* Parses a lower-bounded interval `n..` and gets the lower bound.
4343
*/
4444
bindingset[arg]
45-
private int parseLowerBound(string arg) {
46-
result = arg.regexpCapture("(-?\\d+)\\.\\.", 1).toInt()
47-
}
45+
int parseLowerBound(string arg) { result = arg.regexpCapture("(-?\\d+)\\.\\.", 1).toInt() }
4846

4947
/**
5048
* Parses an integer constant or interval (bounded or unbounded) that explicitly

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,7 @@ module AccessPath {
4242
* Parses a lower-bounded interval `n..` and gets the lower bound.
4343
*/
4444
bindingset[arg]
45-
private int parseLowerBound(string arg) {
46-
result = arg.regexpCapture("(-?\\d+)\\.\\.", 1).toInt()
47-
}
45+
int parseLowerBound(string arg) { result = arg.regexpCapture("(-?\\d+)\\.\\.", 1).toInt() }
4846

4947
/**
5048
* Parses an integer constant or interval (bounded or unbounded) that explicitly

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

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -268,15 +268,17 @@ private module Cached {
268268
TPositionalParameterPosition(int pos) {
269269
pos = any(Parameter p).getPosition()
270270
or
271-
pos in [0 .. 100] // TODO: remove once `Argument[_]` summaries are replaced with `Argument[i..]`
272-
or
273271
FlowSummaryImplSpecific::ParsePositions::isParsedArgumentPosition(_, pos)
274272
} or
273+
TPositionalParameterLowerBoundPosition(int pos) {
274+
FlowSummaryImplSpecific::ParsePositions::isParsedArgumentLowerBoundPosition(_, pos)
275+
} or
275276
TKeywordParameterPosition(string name) {
276277
name = any(KeywordParameter kp).getName()
277278
or
278279
FlowSummaryImplSpecific::ParsePositions::isParsedKeywordArgumentPosition(_, name)
279-
}
280+
} or
281+
TAnyParameterPosition()
280282
}
281283

282284
import Cached
@@ -468,9 +470,18 @@ class ParameterPosition extends TParameterPosition {
468470
/** Holds if this position represents a positional parameter at position `pos`. */
469471
predicate isPositional(int pos) { this = TPositionalParameterPosition(pos) }
470472

473+
/** Holds if this position represents any positional parameter starting from position `pos`. */
474+
predicate isPositionalLowerBound(int pos) { this = TPositionalParameterLowerBoundPosition(pos) }
475+
471476
/** Holds if this position represents a keyword parameter named `name`. */
472477
predicate isKeyword(string name) { this = TKeywordParameterPosition(name) }
473478

479+
/**
480+
* Holds if this position represents any parameter. This includes both positional
481+
* and named parameters.
482+
*/
483+
predicate isAny() { this = TAnyParameterPosition() }
484+
474485
/** Gets a textual representation of this position. */
475486
string toString() {
476487
this.isSelf() and result = "self"
@@ -479,7 +490,11 @@ class ParameterPosition extends TParameterPosition {
479490
or
480491
exists(int pos | this.isPositional(pos) and result = "position " + pos)
481492
or
493+
exists(int pos | this.isPositionalLowerBound(pos) and result = "position " + pos + "..")
494+
or
482495
exists(string name | this.isKeyword(name) and result = "keyword " + name)
496+
or
497+
this.isAny() and result = "any"
483498
}
484499
}
485500

@@ -518,5 +533,11 @@ predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) {
518533
or
519534
exists(int pos | ppos.isPositional(pos) and apos.isPositional(pos))
520535
or
536+
exists(int pos1, int pos2 |
537+
ppos.isPositionalLowerBound(pos1) and apos.isPositional(pos2) and pos2 >= pos1
538+
)
539+
or
521540
exists(string name | ppos.isKeyword(name) and apos.isKeyword(name))
541+
or
542+
ppos.isAny() and exists(apos)
522543
}

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

Lines changed: 34 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -61,21 +61,32 @@ predicate summaryElement(
6161
*
6262
* This covers all the Ruby-specific components of a flow summary.
6363
*/
64-
bindingset[c]
6564
SummaryComponent interpretComponentSpecific(AccessPathToken c) {
66-
c = "Argument[_]" and
67-
result = FlowSummary::SummaryComponent::argument(any(ParameterPosition pos | pos.isPositional(_)))
68-
or
69-
c = "ArrayElement" and
70-
result = FlowSummary::SummaryComponent::arrayElementAny()
71-
or
72-
c = "ArrayElement[?]" and
73-
result = FlowSummary::SummaryComponent::arrayElementUnknown()
65+
c.getName() = "Argument" and
66+
exists(string arg, ParameterPosition ppos |
67+
arg = c.getAnArgument() and
68+
result = FlowSummary::SummaryComponent::argument(ppos)
69+
|
70+
arg = "any" and
71+
ppos.isAny()
72+
or
73+
ppos.isPositionalLowerBound(AccessPath::parseLowerBound(arg))
74+
)
7475
or
75-
exists(int i |
76-
c.getName() = "ArrayElement" and
77-
i = AccessPath::parseInt(c.getAnArgument()) and
78-
result = FlowSummary::SummaryComponent::arrayElementKnown(i)
76+
c.getName() = "ArrayElement" and
77+
(
78+
c.getNumArgument() = 0 and
79+
result = FlowSummary::SummaryComponent::arrayElementAny()
80+
or
81+
exists(string arg | arg = c.getAnArgument() |
82+
arg = "?" and
83+
result = FlowSummary::SummaryComponent::arrayElementUnknown()
84+
or
85+
exists(int i |
86+
i = AccessPath::parseInt(c.getAnArgument()) and
87+
result = FlowSummary::SummaryComponent::arrayElementKnown(i)
88+
)
89+
)
7990
)
8091
}
8192

@@ -201,6 +212,11 @@ module ParsePositions {
201212
i = AccessPath::parseInt(c)
202213
}
203214

215+
predicate isParsedArgumentLowerBoundPosition(string c, int i) {
216+
isArgBody(c) and
217+
i = AccessPath::parseLowerBound(c)
218+
}
219+
204220
predicate isParsedKeywordParameterPosition(string c, string paramName) {
205221
isParamBody(c) and
206222
c = paramName + ":"
@@ -238,6 +254,11 @@ ParameterPosition parseArgBody(string s) {
238254
result.isPositional(i)
239255
)
240256
or
257+
exists(int i |
258+
ParsePositions::isParsedArgumentLowerBoundPosition(s, i) and
259+
result.isPositionalLowerBound(i)
260+
)
261+
or
241262
exists(string name |
242263
ParsePositions::isParsedKeywordArgumentPosition(s, name) and
243264
result.isKeyword(name)

‎ruby/ql/lib/codeql/ruby/frameworks/Files.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,7 @@ module File {
390390
}
391391

392392
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
393-
input = "Argument[_]" and
393+
input = "Argument[0..]" and
394394
output = "ReturnValue" and
395395
preservesValue = false
396396
}

‎ruby/ql/lib/codeql/ruby/frameworks/core/Array.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -507,7 +507,7 @@ module Array {
507507
ConcatSummary() { this = "concat" }
508508

509509
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
510-
input = "Argument[_].ArrayElement" and
510+
input = "Argument[0..].ArrayElement" and
511511
output = "Argument[self].ArrayElement[?]" and
512512
preservesValue = true
513513
}

‎ruby/ql/lib/codeql/ruby/frameworks/core/String.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ module String {
152152
}
153153

154154
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
155-
input = ["Argument[self]", "Argument[_]"] and
155+
input = "Argument[self,0..]" and
156156
output = ["ReturnValue", "Argument[self]"] and
157157
preservesValue = false
158158
}

‎ruby/ql/test/library-tests/dataflow/summaries/Summaries.expected

Lines changed: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,10 @@ edges
1919
| summaries.rb:1:11:1:36 | call to identity : | summaries.rb:37:36:37:42 | tainted |
2020
| summaries.rb:1:11:1:36 | call to identity : | summaries.rb:37:36:37:42 | tainted |
2121
| summaries.rb:1:11:1:36 | call to identity : | summaries.rb:51:24:51:30 | tainted : |
22-
| summaries.rb:1:11:1:36 | call to identity : | summaries.rb:54:23:54:29 | tainted : |
22+
| summaries.rb:1:11:1:36 | call to identity : | summaries.rb:54:22:54:28 | tainted : |
23+
| summaries.rb:1:11:1:36 | call to identity : | summaries.rb:55:17:55:23 | tainted : |
24+
| summaries.rb:1:11:1:36 | call to identity : | summaries.rb:58:32:58:38 | tainted : |
25+
| summaries.rb:1:11:1:36 | call to identity : | summaries.rb:60:23:60:29 | tainted : |
2326
| summaries.rb:1:20:1:36 | call to source : | summaries.rb:1:11:1:36 | call to identity : |
2427
| summaries.rb:1:20:1:36 | call to source : | summaries.rb:1:11:1:36 | call to identity : |
2528
| summaries.rb:4:12:7:3 | call to apply_block : | summaries.rb:9:6:9:13 | tainted2 |
@@ -52,10 +55,13 @@ edges
5255
| summaries.rb:44:8:44:8 | t : | summaries.rb:44:8:44:27 | call to matchedByNameRcv |
5356
| summaries.rb:48:24:48:41 | call to source : | summaries.rb:48:8:48:42 | call to preserveTaint |
5457
| summaries.rb:51:24:51:30 | tainted : | summaries.rb:51:6:51:31 | call to namedArg |
55-
| summaries.rb:54:23:54:29 | tainted : | summaries.rb:54:40:54:40 | x : |
56-
| summaries.rb:54:40:54:40 | x : | summaries.rb:55:8:55:8 | x |
57-
| summaries.rb:62:24:62:53 | call to source : | summaries.rb:62:8:62:54 | call to preserveTaint |
58-
| summaries.rb:65:26:65:56 | call to source : | summaries.rb:65:8:65:57 | call to preserveTaint |
58+
| summaries.rb:54:22:54:28 | tainted : | summaries.rb:54:6:54:29 | call to anyArg |
59+
| summaries.rb:55:17:55:23 | tainted : | summaries.rb:55:6:55:24 | call to anyArg |
60+
| summaries.rb:58:32:58:38 | tainted : | summaries.rb:58:6:58:39 | call to anyPositionFromOne |
61+
| summaries.rb:60:23:60:29 | tainted : | summaries.rb:60:40:60:40 | x : |
62+
| summaries.rb:60:40:60:40 | x : | summaries.rb:61:8:61:8 | x |
63+
| summaries.rb:68:24:68:53 | call to source : | summaries.rb:68:8:68:54 | call to preserveTaint |
64+
| summaries.rb:71:26:71:56 | call to source : | summaries.rb:71:8:71:57 | call to preserveTaint |
5965
nodes
6066
| summaries.rb:1:11:1:36 | call to identity : | semmle.label | call to identity : |
6167
| summaries.rb:1:11:1:36 | call to identity : | semmle.label | call to identity : |
@@ -112,13 +118,19 @@ nodes
112118
| summaries.rb:48:24:48:41 | call to source : | semmle.label | call to source : |
113119
| summaries.rb:51:6:51:31 | call to namedArg | semmle.label | call to namedArg |
114120
| summaries.rb:51:24:51:30 | tainted : | semmle.label | tainted : |
115-
| summaries.rb:54:23:54:29 | tainted : | semmle.label | tainted : |
116-
| summaries.rb:54:40:54:40 | x : | semmle.label | x : |
117-
| summaries.rb:55:8:55:8 | x | semmle.label | x |
118-
| summaries.rb:62:8:62:54 | call to preserveTaint | semmle.label | call to preserveTaint |
119-
| summaries.rb:62:24:62:53 | call to source : | semmle.label | call to source : |
120-
| summaries.rb:65:8:65:57 | call to preserveTaint | semmle.label | call to preserveTaint |
121-
| summaries.rb:65:26:65:56 | call to source : | semmle.label | call to source : |
121+
| summaries.rb:54:6:54:29 | call to anyArg | semmle.label | call to anyArg |
122+
| summaries.rb:54:22:54:28 | tainted : | semmle.label | tainted : |
123+
| summaries.rb:55:6:55:24 | call to anyArg | semmle.label | call to anyArg |
124+
| summaries.rb:55:17:55:23 | tainted : | semmle.label | tainted : |
125+
| summaries.rb:58:6:58:39 | call to anyPositionFromOne | semmle.label | call to anyPositionFromOne |
126+
| summaries.rb:58:32:58:38 | tainted : | semmle.label | tainted : |
127+
| summaries.rb:60:23:60:29 | tainted : | semmle.label | tainted : |
128+
| summaries.rb:60:40:60:40 | x : | semmle.label | x : |
129+
| summaries.rb:61:8:61:8 | x | semmle.label | x |
130+
| summaries.rb:68:8:68:54 | call to preserveTaint | semmle.label | call to preserveTaint |
131+
| summaries.rb:68:24:68:53 | call to source : | semmle.label | call to source : |
132+
| summaries.rb:71:8:71:57 | call to preserveTaint | semmle.label | call to preserveTaint |
133+
| summaries.rb:71:26:71:56 | call to source : | semmle.label | call to source : |
122134
subpaths
123135
invalidSpecComponent
124136
invalidOutputSpecComponent
@@ -150,9 +162,12 @@ invalidOutputSpecComponent
150162
| summaries.rb:44:8:44:27 | call to matchedByNameRcv | summaries.rb:40:7:40:17 | call to source : | summaries.rb:44:8:44:27 | call to matchedByNameRcv | $@ | summaries.rb:40:7:40:17 | call to source : | call to source : |
151163
| summaries.rb:48:8:48:42 | call to preserveTaint | summaries.rb:48:24:48:41 | call to source : | summaries.rb:48:8:48:42 | call to preserveTaint | $@ | summaries.rb:48:24:48:41 | call to source : | call to source : |
152164
| summaries.rb:51:6:51:31 | call to namedArg | summaries.rb:1:20:1:36 | call to source : | summaries.rb:51:6:51:31 | call to namedArg | $@ | summaries.rb:1:20:1:36 | call to source : | call to source : |
153-
| summaries.rb:55:8:55:8 | x | summaries.rb:1:20:1:36 | call to source : | summaries.rb:55:8:55:8 | x | $@ | summaries.rb:1:20:1:36 | call to source : | call to source : |
154-
| summaries.rb:62:8:62:54 | call to preserveTaint | summaries.rb:62:24:62:53 | call to source : | summaries.rb:62:8:62:54 | call to preserveTaint | $@ | summaries.rb:62:24:62:53 | call to source : | call to source : |
155-
| summaries.rb:65:8:65:57 | call to preserveTaint | summaries.rb:65:26:65:56 | call to source : | summaries.rb:65:8:65:57 | call to preserveTaint | $@ | summaries.rb:65:26:65:56 | call to source : | call to source : |
165+
| summaries.rb:54:6:54:29 | call to anyArg | summaries.rb:1:20:1:36 | call to source : | summaries.rb:54:6:54:29 | call to anyArg | $@ | summaries.rb:1:20:1:36 | call to source : | call to source : |
166+
| summaries.rb:55:6:55:24 | call to anyArg | summaries.rb:1:20:1:36 | call to source : | summaries.rb:55:6:55:24 | call to anyArg | $@ | summaries.rb:1:20:1:36 | call to source : | call to source : |
167+
| summaries.rb:58:6:58:39 | call to anyPositionFromOne | summaries.rb:1:20:1:36 | call to source : | summaries.rb:58:6:58:39 | call to anyPositionFromOne | $@ | summaries.rb:1:20:1:36 | call to source : | call to source : |
168+
| summaries.rb:61:8:61:8 | x | summaries.rb:1:20:1:36 | call to source : | summaries.rb:61:8:61:8 | x | $@ | summaries.rb:1:20:1:36 | call to source : | call to source : |
169+
| summaries.rb:68:8:68:54 | call to preserveTaint | summaries.rb:68:24:68:53 | call to source : | summaries.rb:68:8:68:54 | call to preserveTaint | $@ | summaries.rb:68:24:68:53 | call to source : | call to source : |
170+
| summaries.rb:71:8:71:57 | call to preserveTaint | summaries.rb:71:26:71:56 | call to source : | summaries.rb:71:8:71:57 | call to preserveTaint | $@ | summaries.rb:71:26:71:56 | call to source : | call to source : |
156171
warning
157172
| CSV type row should have 5 columns but has 2: test;TooFewColumns |
158173
| CSV type row should have 5 columns but has 8: test;TooManyColumns;;;Member[Foo].Instance;too;many;columns |

0 commit comments

Comments
 (0)