Skip to content

Commit 38999a3

Browse files
hmachmac
committed
Ruby: Model ActionController::Parameters
Add flow summaries for methods on ActionController::Parameters, which mostly propagate taint from receiver to return value.
1 parent 88baf08 commit 38999a3

File tree

6 files changed

+583
-0
lines changed

6 files changed

+583
-0
lines changed

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

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,3 +357,123 @@ private class ActionControllerProtectFromForgeryCall extends CsrfProtectionSetti
357357
if this.getWithValueText() = "exception" then result = true else result = false
358358
}
359359
}
360+
361+
private module ParamsSummaries {
362+
private import codeql.ruby.dataflow.FlowSummary
363+
364+
/**
365+
* An instance of `ActionController::Parameters`, including those returned
366+
* from method calls on other instances.
367+
*/
368+
private class ParamsInstance extends DataFlow::Node {
369+
ParamsInstance() {
370+
this.asExpr().getExpr() instanceof ParamsCall
371+
or
372+
exists(DataFlow::CallNode call | call = this |
373+
call.getReceiver() instanceof ParamsInstance and
374+
call.getMethodName() = paramsMethodReturningParamsInstance()
375+
)
376+
or
377+
exists(DataFlow::LocalSourceNode prev | prev.flowsTo(this))
378+
}
379+
}
380+
381+
/**
382+
* Methods on `ActionController::Parameters` that return an instance of
383+
* `ActionController::Parameters`.
384+
*/
385+
string paramsMethodReturningParamsInstance() {
386+
result =
387+
[
388+
"concat", "concat!", "compact_blank", "deep_dup", "deep_transform_keys", "delete_if",
389+
// dig doesn't always return a Parameters instance, but it will if the
390+
// given key refers to a nested hash parameter.
391+
"dig", "each", "each_key", "each_pair", "each_value", "except", "keep_if", "merge",
392+
"merge!", "permit", "reject", "reject!", "reverse_merge", "reverse_merge!", "select",
393+
"select!", "slice", "slice!", "transform_keys", "transform_keys!", "transform_values",
394+
"transform_values!", "with_defaults", "with_defaults!"
395+
]
396+
}
397+
398+
/**
399+
* Methods on `ActionController::Parameters` that propagate taint from
400+
* receiver to return value.
401+
*/
402+
string methodReturnsTaintFromSelf() {
403+
result =
404+
[
405+
"as_json", "permit", "require", "required", "deep_dup", "deep_transform_keys",
406+
"deep_transform_keys!", "delete_if", "extract!", "keep_if", "select", "select!", "reject",
407+
"reject!", "to_h", "to_hash", "to_query", "to_param", "to_unsafe_h", "to_unsafe_hash",
408+
"transform_keys", "transform_keys!", "transform_values", "transform_values!", "values_at"
409+
]
410+
}
411+
412+
/**
413+
* Methods on `ActionController::Parameters` which propagate taint from
414+
* receiver to return value.
415+
*/
416+
private class TaintReturnFromSelf extends SummarizedCallable {
417+
// TODO: better name?
418+
TaintReturnFromSelf() { this = "ActionController::Parameters#<various>" }
419+
420+
override MethodCall getACall() {
421+
any(ParamsInstance i).asExpr().getExpr() = result.getReceiver() and
422+
result.getMethodName() = methodReturnsTaintFromSelf()
423+
}
424+
425+
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
426+
input = "Argument[self]" and
427+
output = "ReturnValue" and
428+
preservesValue = false
429+
}
430+
}
431+
432+
/**
433+
* `#merge`
434+
* Returns a new ActionController::Parameters with all keys from other_hash merged into current hash.
435+
* `#reverse_merge`
436+
* `#with_defaults`
437+
* Returns a new ActionController::Parameters with all keys from current hash merged into other_hash.
438+
*/
439+
private class MergeSummary extends SummarizedCallable {
440+
MergeSummary() { this = "ActionController::Parameters#merge" }
441+
442+
override MethodCall getACall() {
443+
result.getMethodName() = ["merge", "reverse_merge", "with_defaults"] and
444+
exists(ParamsInstance i |
445+
i.asExpr().getExpr() = [result.getReceiver(), result.getArgument(0)]
446+
)
447+
}
448+
449+
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
450+
input = ["Argument[self]", "Argument[0]"] and
451+
output = "ReturnValue" and
452+
preservesValue = false
453+
}
454+
}
455+
456+
/**
457+
* `#merge!`
458+
* Returns current ActionController::Parameters instance with current hash merged into other_hash.
459+
* `#reverse_merge!`
460+
* `#with_defaults!`
461+
* Returns a new ActionController::Parameters with all keys from current hash merged into other_hash.
462+
*/
463+
private class MergeBangSummary extends SummarizedCallable {
464+
MergeBangSummary() { this = "ActionController::Parameters#merge!" }
465+
466+
override MethodCall getACall() {
467+
result.getMethodName() = ["merge!", "reverse_merge!", "with_defaults!"] and
468+
exists(ParamsInstance i |
469+
i.asExpr().getExpr() = [result.getReceiver(), result.getArgument(0)]
470+
)
471+
}
472+
473+
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
474+
input = ["Argument[self]", "Argument[0]"] and
475+
output = ["ReturnValue", "Argument[self]"] and
476+
preservesValue = false
477+
}
478+
}
479+
}

‎ruby/ql/test/library-tests/frameworks/ActionController.expected

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
actionControllerControllerClasses
2+
| action_controller/params_flow.rb:1:1:151:3 | MyController |
23
| active_record/ActiveRecord.rb:23:1:39:3 | FooController |
34
| active_record/ActiveRecord.rb:41:1:64:3 | BarController |
45
| active_record/ActiveRecord.rb:66:1:94:3 | BazController |
@@ -9,6 +10,39 @@ actionControllerControllerClasses
910
| app/controllers/posts_controller.rb:1:1:10:3 | PostsController |
1011
| app/controllers/users/notifications_controller.rb:2:3:5:5 | NotificationsController |
1112
actionControllerActionMethods
13+
| action_controller/params_flow.rb:2:3:4:5 | m1 |
14+
| action_controller/params_flow.rb:6:3:8:5 | m2 |
15+
| action_controller/params_flow.rb:10:3:12:5 | m2 |
16+
| action_controller/params_flow.rb:14:3:16:5 | m3 |
17+
| action_controller/params_flow.rb:18:3:20:5 | m4 |
18+
| action_controller/params_flow.rb:22:3:24:5 | m5 |
19+
| action_controller/params_flow.rb:26:3:28:5 | m6 |
20+
| action_controller/params_flow.rb:30:3:32:5 | m7 |
21+
| action_controller/params_flow.rb:34:3:36:5 | m8 |
22+
| action_controller/params_flow.rb:38:3:40:5 | m9 |
23+
| action_controller/params_flow.rb:42:3:44:5 | m10 |
24+
| action_controller/params_flow.rb:46:3:48:5 | m11 |
25+
| action_controller/params_flow.rb:50:3:52:5 | m12 |
26+
| action_controller/params_flow.rb:54:3:56:5 | m13 |
27+
| action_controller/params_flow.rb:58:3:60:5 | m14 |
28+
| action_controller/params_flow.rb:62:3:64:5 | m15 |
29+
| action_controller/params_flow.rb:66:3:68:5 | m16 |
30+
| action_controller/params_flow.rb:70:3:72:5 | m17 |
31+
| action_controller/params_flow.rb:74:3:76:5 | m18 |
32+
| action_controller/params_flow.rb:78:3:80:5 | m19 |
33+
| action_controller/params_flow.rb:82:3:84:5 | m20 |
34+
| action_controller/params_flow.rb:86:3:88:5 | m21 |
35+
| action_controller/params_flow.rb:90:3:92:5 | m22 |
36+
| action_controller/params_flow.rb:94:3:96:5 | m23 |
37+
| action_controller/params_flow.rb:98:3:100:5 | m24 |
38+
| action_controller/params_flow.rb:102:3:104:5 | m25 |
39+
| action_controller/params_flow.rb:106:3:108:5 | m26 |
40+
| action_controller/params_flow.rb:110:3:113:5 | m27 |
41+
| action_controller/params_flow.rb:115:3:118:5 | m28 |
42+
| action_controller/params_flow.rb:120:3:123:5 | m29 |
43+
| action_controller/params_flow.rb:125:3:132:5 | m30 |
44+
| action_controller/params_flow.rb:134:3:141:5 | m31 |
45+
| action_controller/params_flow.rb:143:3:150:5 | m32 |
1246
| active_record/ActiveRecord.rb:27:3:38:5 | some_request_handler |
1347
| active_record/ActiveRecord.rb:42:3:47:5 | some_other_request_handler |
1448
| active_record/ActiveRecord.rb:49:3:63:5 | safe_paths |
@@ -35,6 +69,48 @@ actionControllerActionMethods
3569
| app/controllers/posts_controller.rb:8:3:9:5 | upvote |
3670
| app/controllers/users/notifications_controller.rb:3:5:4:7 | mark_as_read |
3771
paramsCalls
72+
| action_controller/params_flow.rb:3:10:3:15 | call to params |
73+
| action_controller/params_flow.rb:7:10:7:15 | call to params |
74+
| action_controller/params_flow.rb:11:10:11:15 | call to params |
75+
| action_controller/params_flow.rb:15:10:15:15 | call to params |
76+
| action_controller/params_flow.rb:19:10:19:15 | call to params |
77+
| action_controller/params_flow.rb:23:10:23:15 | call to params |
78+
| action_controller/params_flow.rb:27:10:27:15 | call to params |
79+
| action_controller/params_flow.rb:31:10:31:15 | call to params |
80+
| action_controller/params_flow.rb:35:10:35:15 | call to params |
81+
| action_controller/params_flow.rb:39:10:39:15 | call to params |
82+
| action_controller/params_flow.rb:43:10:43:15 | call to params |
83+
| action_controller/params_flow.rb:47:10:47:15 | call to params |
84+
| action_controller/params_flow.rb:51:10:51:15 | call to params |
85+
| action_controller/params_flow.rb:55:10:55:15 | call to params |
86+
| action_controller/params_flow.rb:59:10:59:15 | call to params |
87+
| action_controller/params_flow.rb:63:10:63:15 | call to params |
88+
| action_controller/params_flow.rb:67:10:67:15 | call to params |
89+
| action_controller/params_flow.rb:71:10:71:15 | call to params |
90+
| action_controller/params_flow.rb:75:10:75:15 | call to params |
91+
| action_controller/params_flow.rb:79:10:79:15 | call to params |
92+
| action_controller/params_flow.rb:83:10:83:15 | call to params |
93+
| action_controller/params_flow.rb:87:10:87:15 | call to params |
94+
| action_controller/params_flow.rb:91:10:91:15 | call to params |
95+
| action_controller/params_flow.rb:95:10:95:15 | call to params |
96+
| action_controller/params_flow.rb:99:10:99:15 | call to params |
97+
| action_controller/params_flow.rb:103:10:103:15 | call to params |
98+
| action_controller/params_flow.rb:107:10:107:15 | call to params |
99+
| action_controller/params_flow.rb:111:10:111:15 | call to params |
100+
| action_controller/params_flow.rb:112:23:112:28 | call to params |
101+
| action_controller/params_flow.rb:116:10:116:15 | call to params |
102+
| action_controller/params_flow.rb:117:31:117:36 | call to params |
103+
| action_controller/params_flow.rb:121:10:121:15 | call to params |
104+
| action_controller/params_flow.rb:122:31:122:36 | call to params |
105+
| action_controller/params_flow.rb:126:10:126:15 | call to params |
106+
| action_controller/params_flow.rb:127:24:127:29 | call to params |
107+
| action_controller/params_flow.rb:130:14:130:19 | call to params |
108+
| action_controller/params_flow.rb:135:10:135:15 | call to params |
109+
| action_controller/params_flow.rb:136:32:136:37 | call to params |
110+
| action_controller/params_flow.rb:139:22:139:27 | call to params |
111+
| action_controller/params_flow.rb:144:10:144:15 | call to params |
112+
| action_controller/params_flow.rb:145:32:145:37 | call to params |
113+
| action_controller/params_flow.rb:148:22:148:27 | call to params |
38114
| active_record/ActiveRecord.rb:28:30:28:35 | call to params |
39115
| active_record/ActiveRecord.rb:29:29:29:34 | call to params |
40116
| active_record/ActiveRecord.rb:30:31:30:36 | call to params |
@@ -65,6 +141,48 @@ paramsCalls
65141
| app/controllers/foo/bars_controller.rb:22:10:22:15 | call to params |
66142
| app/views/foo/bars/show.html.erb:5:9:5:14 | call to params |
67143
paramsSources
144+
| action_controller/params_flow.rb:3:10:3:15 | call to params |
145+
| action_controller/params_flow.rb:7:10:7:15 | call to params |
146+
| action_controller/params_flow.rb:11:10:11:15 | call to params |
147+
| action_controller/params_flow.rb:15:10:15:15 | call to params |
148+
| action_controller/params_flow.rb:19:10:19:15 | call to params |
149+
| action_controller/params_flow.rb:23:10:23:15 | call to params |
150+
| action_controller/params_flow.rb:27:10:27:15 | call to params |
151+
| action_controller/params_flow.rb:31:10:31:15 | call to params |
152+
| action_controller/params_flow.rb:35:10:35:15 | call to params |
153+
| action_controller/params_flow.rb:39:10:39:15 | call to params |
154+
| action_controller/params_flow.rb:43:10:43:15 | call to params |
155+
| action_controller/params_flow.rb:47:10:47:15 | call to params |
156+
| action_controller/params_flow.rb:51:10:51:15 | call to params |
157+
| action_controller/params_flow.rb:55:10:55:15 | call to params |
158+
| action_controller/params_flow.rb:59:10:59:15 | call to params |
159+
| action_controller/params_flow.rb:63:10:63:15 | call to params |
160+
| action_controller/params_flow.rb:67:10:67:15 | call to params |
161+
| action_controller/params_flow.rb:71:10:71:15 | call to params |
162+
| action_controller/params_flow.rb:75:10:75:15 | call to params |
163+
| action_controller/params_flow.rb:79:10:79:15 | call to params |
164+
| action_controller/params_flow.rb:83:10:83:15 | call to params |
165+
| action_controller/params_flow.rb:87:10:87:15 | call to params |
166+
| action_controller/params_flow.rb:91:10:91:15 | call to params |
167+
| action_controller/params_flow.rb:95:10:95:15 | call to params |
168+
| action_controller/params_flow.rb:99:10:99:15 | call to params |
169+
| action_controller/params_flow.rb:103:10:103:15 | call to params |
170+
| action_controller/params_flow.rb:107:10:107:15 | call to params |
171+
| action_controller/params_flow.rb:111:10:111:15 | call to params |
172+
| action_controller/params_flow.rb:112:23:112:28 | call to params |
173+
| action_controller/params_flow.rb:116:10:116:15 | call to params |
174+
| action_controller/params_flow.rb:117:31:117:36 | call to params |
175+
| action_controller/params_flow.rb:121:10:121:15 | call to params |
176+
| action_controller/params_flow.rb:122:31:122:36 | call to params |
177+
| action_controller/params_flow.rb:126:10:126:15 | call to params |
178+
| action_controller/params_flow.rb:127:24:127:29 | call to params |
179+
| action_controller/params_flow.rb:130:14:130:19 | call to params |
180+
| action_controller/params_flow.rb:135:10:135:15 | call to params |
181+
| action_controller/params_flow.rb:136:32:136:37 | call to params |
182+
| action_controller/params_flow.rb:139:22:139:27 | call to params |
183+
| action_controller/params_flow.rb:144:10:144:15 | call to params |
184+
| action_controller/params_flow.rb:145:32:145:37 | call to params |
185+
| action_controller/params_flow.rb:148:22:148:27 | call to params |
68186
| active_record/ActiveRecord.rb:28:30:28:35 | call to params |
69187
| active_record/ActiveRecord.rb:29:29:29:34 | call to params |
70188
| active_record/ActiveRecord.rb:30:31:30:36 | call to params |

0 commit comments

Comments
 (0)