Skip to content

Commit 3f403f0

Browse files
hmachmac
authored
Merge pull request #10700 from hmac/activesupport
Ruby: Model some ActiveSupport methods
2 parents 85790fc + 368ce69 commit 3f403f0

File tree

13 files changed

+1233
-356
lines changed

13 files changed

+1233
-356
lines changed

‎ruby/ql/lib/change-notes/2022-10-18-activesupport-flow.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Taint flow is now tracked through extension methods on `Hash`, `String` and
5+
`Object` provided by `ActiveSupport`.

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

Lines changed: 117 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,17 @@ module ActiveSupport {
2424
*/
2525
module String {
2626
/**
27-
* A call to `String#constantize`, which tries to find a declared constant with the given name.
28-
* Passing user input to this method may result in instantiation of arbitrary Ruby classes.
27+
* A call to `String#constantize` or `String#safe_constantize`, which
28+
* tries to find a declared constant with the given name.
29+
* Passing user input to this method may result in instantiation of
30+
* arbitrary Ruby classes.
2931
*/
3032
class Constantize extends CodeExecution::Range, DataFlow::CallNode {
3133
// We treat this an `UnknownMethodCall` in order to match every call to `constantize` that isn't overridden.
3234
// We can't (yet) rely on API Graphs or dataflow to tell us that the receiver is a String.
3335
Constantize() {
34-
this.asExpr().getExpr().(UnknownMethodCall).getMethodName() = "constantize"
36+
this.asExpr().getExpr().(UnknownMethodCall).getMethodName() =
37+
["constantize", "safe_constantize"]
3538
}
3639

3740
override DataFlow::Node getCode() { result = this.getReceiver() }
@@ -49,9 +52,11 @@ module ActiveSupport {
4952
override MethodCall getACall() {
5053
result.getMethodName() =
5154
[
52-
"camelize", "camelcase", "classify", "dasherize", "deconstantize", "demodulize",
53-
"foreign_key", "humanize", "indent", "parameterize", "pluralize", "singularize",
54-
"squish", "strip_heredoc", "tableize", "titlecase", "titleize", "underscore",
55+
"at", "camelize", "camelcase", "classify", "dasherize", "deconstantize", "demodulize",
56+
"first", "foreign_key", "from", "html_safe", "humanize", "indent", "indent!",
57+
"inquiry", "last", "mb_chars", "parameterize", "pluralize", "remove", "remove!",
58+
"singularize", "squish", "squish!", "strip_heredoc", "tableize", "titlecase",
59+
"titleize", "to", "truncate", "truncate_bytes", "truncate_words", "underscore",
5560
"upcase_first"
5661
]
5762
}
@@ -62,6 +67,112 @@ module ActiveSupport {
6267
}
6368
}
6469

70+
/**
71+
* Extensions to the `Object` class.
72+
*/
73+
module Object {
74+
/** Flow summary for methods which can return the receiver. */
75+
private class IdentitySummary extends SimpleSummarizedCallable {
76+
IdentitySummary() { this = ["presence", "deep_dup"] }
77+
78+
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
79+
input = "Argument[self]" and
80+
output = "ReturnValue" and
81+
preservesValue = true
82+
}
83+
}
84+
}
85+
86+
/**
87+
* Extensions to the `Hash` class.
88+
*/
89+
module Hash {
90+
private class WithIndifferentAccessSummary extends SimpleSummarizedCallable {
91+
WithIndifferentAccessSummary() { this = "with_indifferent_access" }
92+
93+
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
94+
input = "Argument[self].Element[any]" and
95+
output = "ReturnValue.Element[any]" and
96+
preservesValue = true
97+
}
98+
}
99+
100+
private class TransformSummary extends SimpleSummarizedCallable {
101+
TransformSummary() {
102+
this =
103+
[
104+
"stringify_keys", "to_options", "symbolize_keys", "deep_stringify_keys",
105+
"deep_symbolize_keys", "with_indifferent_access"
106+
]
107+
}
108+
109+
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
110+
input = "Argument[self].Element[any]" and
111+
output = "ReturnValue.Element[?]" and
112+
preservesValue = true
113+
}
114+
}
115+
116+
private string getExtractComponent(MethodCall mc, int i) {
117+
mc.getMethodName() = "extract!" and
118+
result = DataFlow::Content::getKnownElementIndex(mc.getArgument(i)).serialize()
119+
}
120+
121+
/**
122+
* A flow summary for `Hash#extract!`. This method removes the key/value pairs
123+
* matching the given keys from the receiver and returns them (as a Hash).
124+
*
125+
* Example:
126+
*
127+
* ```rb
128+
* hash = { a: 1, b: 2, c: 3, d: 4 }
129+
* hash.extract!(:a, :b) # => {:a=>1, :b=>2}
130+
* hash # => {:c=>3, :d=>4}
131+
* ```
132+
*
133+
* There is value flow from elements corresponding to keys in the
134+
* arguments (`:a` and `:b` in the example) to elements in
135+
* the return value.
136+
* There is also value flow from any element corresponding to a key _not_
137+
* mentioned in the arguments to an element in `self`, including elements
138+
* at unknown keys.
139+
*/
140+
private class ExtractSummary extends SummarizedCallable {
141+
MethodCall mc;
142+
143+
ExtractSummary() {
144+
mc.getMethodName() = "extract!" and
145+
this =
146+
"extract!(" +
147+
concat(int i, string s | s = getExtractComponent(mc, i) | s, "," order by i) + ")"
148+
}
149+
150+
final override MethodCall getACall() { result = mc }
151+
152+
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
153+
(
154+
exists(string s | s = getExtractComponent(mc, _) |
155+
input = "Argument[self].Element[" + s + "!]" and
156+
output = "ReturnValue.Element[" + s + "!]"
157+
)
158+
or
159+
// Argument[self].WithoutElement[:a!, :b!].WithElement[any] means
160+
// "an element of self whose key is not :a or :b, including elements
161+
// with unknown keys"
162+
input =
163+
"Argument[self]" +
164+
concat(int i, string s |
165+
s = getExtractComponent(mc, i)
166+
|
167+
".WithoutElement[" + s + "!]" order by i
168+
) + ".WithElement[any]" and
169+
output = "Argument[self]"
170+
) and
171+
preservesValue = true
172+
}
173+
}
174+
}
175+
65176
/**
66177
* Extensions to the `Enumerable` module.
67178
*/

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

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -244,18 +244,20 @@ module Hash {
244244
}
245245

246246
private string getExceptComponent(MethodCall mc, int i) {
247-
mc.getMethodName() = "except" and
247+
mc.getMethodName() = ["except", "except!"] and
248248
result = DataFlow::Content::getKnownElementIndex(mc.getArgument(i)).serialize()
249249
}
250250

251251
private class ExceptSummary extends SummarizedCallable {
252252
MethodCall mc;
253253

254254
ExceptSummary() {
255-
mc.getMethodName() = "except" and
255+
// except! is an ActiveSupport extension
256+
// https://api.rubyonrails.org/classes/Hash.html#method-i-except-21
257+
mc.getMethodName() = ["except", "except!"] and
256258
this =
257-
"except(" + concat(int i, string s | s = getExceptComponent(mc, i) | s, "," order by i) +
258-
")"
259+
mc.getMethodName() + "(" +
260+
concat(int i, string s | s = getExceptComponent(mc, i) | s, "," order by i) + ")"
259261
}
260262

261263
final override MethodCall getACallSimple() { result = mc }
@@ -268,7 +270,11 @@ module Hash {
268270
|
269271
".WithoutElement[" + s + "!]" order by i
270272
) + ".WithElement[any]" and
271-
output = "ReturnValue" and
273+
(
274+
if mc.getMethodName() = "except!"
275+
then output = ["ReturnValue", "Argument[self]"]
276+
else output = "ReturnValue"
277+
) and
272278
preservesValue = true
273279
}
274280
}
@@ -331,7 +337,11 @@ private class FetchValuesUnknownSummary extends FetchValuesSummary {
331337
}
332338

333339
private class MergeSummary extends SimpleSummarizedCallable {
334-
MergeSummary() { this = "merge" }
340+
MergeSummary() {
341+
// deep_merge is an ActiveSupport extension
342+
// https://api.rubyonrails.org/classes/Hash.html#method-i-deep_merge
343+
this = ["merge", "deep_merge"]
344+
}
335345

336346
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
337347
(
@@ -346,7 +356,11 @@ private class MergeSummary extends SimpleSummarizedCallable {
346356
}
347357

348358
private class MergeBangSummary extends SimpleSummarizedCallable {
349-
MergeBangSummary() { this = ["merge!", "update"] }
359+
MergeBangSummary() {
360+
// deep_merge! is an ActiveSupport extension
361+
// https://api.rubyonrails.org/classes/Hash.html#method-i-deep_merge-21
362+
this = ["merge!", "deep_merge!", "update"]
363+
}
350364

351365
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
352366
(

‎ruby/ql/test/library-tests/dataflow/array-flow/type-tracking-array-flow.ql‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,6 @@
1+
// This test flags any difference in flow between the type-tracking and dataflow
2+
// libraries. New results in this query do not necessarily indicate a problem,
3+
// only that type-tracking cannot follow the flow in your test. If the dataflow
4+
// test (`array-flow.ql`) shows no failures, then that may be sufficient
5+
// (depending on your use case).
16
import TestUtilities.InlineTypeTrackingFlowTest

0 commit comments

Comments
 (0)