Skip to content

Commit ce54eb3

Browse files
asgerfasgerf
committed
Ruby: Add Argument[foo:] syntax for keyword arguments
1 parent c923b9b commit ce54eb3

File tree

6 files changed

+74
-2
lines changed

6 files changed

+74
-2
lines changed

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,8 @@ private module Cached {
257257
name = any(KeywordParameter kp).getName()
258258
or
259259
exists(any(Call c).getKeywordArgument(name))
260+
or
261+
FlowSummaryImplSpecific::ParsePositions::isParsedKeywordParameterPosition(_, name)
260262
}
261263

262264
cached
@@ -270,7 +272,11 @@ private module Cached {
270272
or
271273
FlowSummaryImplSpecific::ParsePositions::isParsedArgumentPosition(_, pos)
272274
} or
273-
TKeywordParameterPosition(string name) { name = any(KeywordParameter kp).getName() }
275+
TKeywordParameterPosition(string name) {
276+
name = any(KeywordParameter kp).getName()
277+
or
278+
FlowSummaryImplSpecific::ParsePositions::isParsedKeywordArgumentPosition(_, name)
279+
}
274280
}
275281

276282
import Cached

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,16 @@ module ParsePositions {
166166
isArgBody(c) and
167167
i = AccessPath::parseInt(c)
168168
}
169+
170+
predicate isParsedKeywordParameterPosition(string c, string paramName) {
171+
isParamBody(c) and
172+
c = paramName + ":"
173+
}
174+
175+
predicate isParsedKeywordArgumentPosition(string c, string paramName) {
176+
isArgBody(c) and
177+
c = paramName + ":"
178+
}
169179
}
170180

171181
/** Gets the argument position obtained by parsing `X` in `Parameter[X]`. */
@@ -175,6 +185,11 @@ ArgumentPosition parseParamBody(string s) {
175185
result.isPositional(i)
176186
)
177187
or
188+
exists(string name |
189+
ParsePositions::isParsedKeywordParameterPosition(s, name) and
190+
result.isKeyword(name)
191+
)
192+
or
178193
s = "self" and
179194
result.isSelf()
180195
or
@@ -189,6 +204,11 @@ ParameterPosition parseArgBody(string s) {
189204
result.isPositional(i)
190205
)
191206
or
207+
exists(string name |
208+
ParsePositions::isParsedKeywordArgumentPosition(s, name) and
209+
result.isKeyword(name)
210+
)
211+
or
192212
s = "self" and
193213
result.isSelf()
194214
or

‎ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModelsSpecific.qll

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,5 +176,9 @@ predicate isExtraValidTokenArgumentInIdentifyingAccessPath(string name, string a
176176
exists(argument)
177177
or
178178
name = ["Argument", "Parameter"] and
179-
argument = ["self", "block"]
179+
(
180+
argument = ["self", "block"]
181+
or
182+
argument.regexpMatch("\\w+:") // keyword argument
183+
)
180184
}

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ edges
1010
| summaries.rb:1:11:1:26 | call to identity : | summaries.rb:35:16:35:22 | tainted |
1111
| summaries.rb:1:11:1:26 | call to identity : | summaries.rb:36:21:36:27 | tainted |
1212
| summaries.rb:1:11:1:26 | call to identity : | summaries.rb:37:36:37:42 | tainted |
13+
| summaries.rb:1:11:1:26 | call to identity : | summaries.rb:51:24:51:30 | tainted : |
14+
| summaries.rb:1:11:1:26 | call to identity : | summaries.rb:54:23:54:29 | tainted : |
1315
| summaries.rb:1:20:1:26 | "taint" : | summaries.rb:1:11:1:26 | call to identity : |
1416
| summaries.rb:4:12:7:3 | call to apply_block : | summaries.rb:9:6:9:13 | tainted2 |
1517
| summaries.rb:4:24:4:30 | tainted : | summaries.rb:4:12:7:3 | call to apply_block : |
@@ -32,6 +34,11 @@ edges
3234
| summaries.rb:42:24:42:24 | t : | summaries.rb:42:8:42:25 | call to matchedByName |
3335
| summaries.rb:44:8:44:8 | t : | summaries.rb:44:8:44:27 | call to matchedByNameRcv |
3436
| summaries.rb:48:24:48:30 | "taint" : | summaries.rb:48:8:48:31 | call to preserveTaint |
37+
| summaries.rb:51:24:51:30 | tainted : | summaries.rb:51:6:51:31 | call to namedArg |
38+
| summaries.rb:54:23:54:29 | tainted : | summaries.rb:54:40:54:40 | x : |
39+
| summaries.rb:54:40:54:40 | x : | summaries.rb:55:8:55:8 | x |
40+
| summaries.rb:62:24:62:30 | "taint" : | summaries.rb:62:8:62:31 | call to preserveTaint |
41+
| summaries.rb:65:26:65:32 | "taint" : | summaries.rb:65:8:65:33 | call to preserveTaint |
3542
nodes
3643
| summaries.rb:1:11:1:26 | call to identity : | semmle.label | call to identity : |
3744
| summaries.rb:1:20:1:26 | "taint" : | semmle.label | "taint" : |
@@ -69,6 +76,15 @@ nodes
6976
| summaries.rb:44:8:44:27 | call to matchedByNameRcv | semmle.label | call to matchedByNameRcv |
7077
| summaries.rb:48:8:48:31 | call to preserveTaint | semmle.label | call to preserveTaint |
7178
| summaries.rb:48:24:48:30 | "taint" : | semmle.label | "taint" : |
79+
| summaries.rb:51:6:51:31 | call to namedArg | semmle.label | call to namedArg |
80+
| summaries.rb:51:24:51:30 | tainted : | semmle.label | tainted : |
81+
| summaries.rb:54:23:54:29 | tainted : | semmle.label | tainted : |
82+
| summaries.rb:54:40:54:40 | x : | semmle.label | x : |
83+
| summaries.rb:55:8:55:8 | x | semmle.label | x |
84+
| summaries.rb:62:8:62:31 | call to preserveTaint | semmle.label | call to preserveTaint |
85+
| summaries.rb:62:24:62:30 | "taint" : | semmle.label | "taint" : |
86+
| summaries.rb:65:8:65:33 | call to preserveTaint | semmle.label | call to preserveTaint |
87+
| summaries.rb:65:26:65:32 | "taint" : | semmle.label | "taint" : |
7288
subpaths
7389
invalidSpecComponent
7490
invalidOutputSpecComponent
@@ -90,6 +106,10 @@ invalidOutputSpecComponent
90106
| summaries.rb:42:8:42:25 | call to matchedByName | summaries.rb:40:7:40:13 | "taint" : | summaries.rb:42:8:42:25 | call to matchedByName | $@ | summaries.rb:40:7:40:13 | "taint" : | "taint" : |
91107
| summaries.rb:44:8:44:27 | call to matchedByNameRcv | summaries.rb:40:7:40:13 | "taint" : | summaries.rb:44:8:44:27 | call to matchedByNameRcv | $@ | summaries.rb:40:7:40:13 | "taint" : | "taint" : |
92108
| summaries.rb:48:8:48:31 | call to preserveTaint | summaries.rb:48:24:48:30 | "taint" : | summaries.rb:48:8:48:31 | call to preserveTaint | $@ | summaries.rb:48:24:48:30 | "taint" : | "taint" : |
109+
| summaries.rb:51:6:51:31 | call to namedArg | summaries.rb:1:20:1:26 | "taint" : | summaries.rb:51:6:51:31 | call to namedArg | $@ | summaries.rb:1:20:1:26 | "taint" : | "taint" : |
110+
| summaries.rb:55:8:55:8 | x | summaries.rb:1:20:1:26 | "taint" : | summaries.rb:55:8:55:8 | x | $@ | summaries.rb:1:20:1:26 | "taint" : | "taint" : |
111+
| summaries.rb:62:8:62:31 | call to preserveTaint | summaries.rb:62:24:62:30 | "taint" : | summaries.rb:62:8:62:31 | call to preserveTaint | $@ | summaries.rb:62:24:62:30 | "taint" : | "taint" : |
112+
| summaries.rb:65:8:65:33 | call to preserveTaint | summaries.rb:65:26:65:32 | "taint" : | summaries.rb:65:8:65:33 | call to preserveTaint | $@ | summaries.rb:65:26:65:32 | "taint" : | "taint" : |
93113
warning
94114
| CSV type row should have 5 columns but has 2: test;TooFewColumns |
95115
| CSV type row should have 5 columns but has 8: test;TooManyColumns;;;Member[Foo].Instance;too;many;columns |

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,11 @@ private class StepsFromModel extends ModelInput::SummaryModelCsv {
7676
";;Member[Foo].Method[onlyWithoutBlock].WithoutBlock;Argument[0];ReturnValue;taint",
7777
";;Member[Foo].Method[onlyWithBlock].WithBlock;Argument[0];ReturnValue;taint",
7878
";;Member[Foo].Method[blockArg].Argument[block].Parameter[0].Method[preserveTaint];Argument[0];ReturnValue;taint",
79+
";;Member[Foo].Method[namedArg];Argument[foo:];ReturnValue;taint",
80+
";;Member[Foo].Method[intoNamedCallback];Argument[0];Argument[foo:].Parameter[0];taint",
81+
";;Member[Foo].Method[intoNamedParameter];Argument[0];Argument[0].Parameter[foo:];taint",
82+
";;Member[Foo].Method[startInNamedCallback].Argument[foo:].Parameter[0].Method[preserveTaint];Argument[0];ReturnValue;taint",
83+
";;Member[Foo].Method[startInNamedParameter].Argument[0].Parameter[foo:].Method[preserveTaint];Argument[0];ReturnValue;taint",
7984
";any;Method[matchedByName];Argument[0];ReturnValue;taint",
8085
";any;Method[matchedByNameRcv];Argument[self];ReturnValue;taint",
8186
]

‎ruby/ql/test/library-tests/dataflow/summaries/summaries.rb

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,3 +47,20 @@ def userDefinedFunction(x, y)
4747
Foo.blockArg do |x|
4848
sink(x.preserveTaint("taint"))
4949
end
50+
51+
sink(Foo.namedArg(foo: tainted))
52+
sink(Foo.namedArg(tainted))
53+
54+
Foo.intoNamedCallback(tainted, foo: ->(x) {
55+
sink(x)
56+
})
57+
Foo.intoNamedParameter(tainted, ->(foo:) {
58+
sink(foo)
59+
})
60+
61+
Foo.startInNamedCallback(foo: ->(x) {
62+
sink(x.preserveTaint("taint"))
63+
})
64+
Foo.startInNamedParameter(->(foo:) {
65+
sink(foo.preserveTaint("taint"))
66+
})

0 commit comments

Comments
 (0)