Skip to content

Commit 3910704

Browse files
authored
Merge pull request #10735 from github/nickrolfe/actionmailer
Ruby: add `ActionMailer#params` as a `RemoteFlowSource`
2 parents 202549b + 078c3e9 commit 3910704

File tree

7 files changed

+92
-0
lines changed

7 files changed

+92
-0
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Calls to `params` in `ActionMailer` classes are now treated as sources of remote user input.

ruby/ql/lib/codeql/ruby/Frameworks.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
private import codeql.ruby.frameworks.Core
66
private import codeql.ruby.frameworks.ActionCable
77
private import codeql.ruby.frameworks.ActionController
8+
private import codeql.ruby.frameworks.ActionMailer
89
private import codeql.ruby.frameworks.ActiveRecord
910
private import codeql.ruby.frameworks.ActiveResource
1011
private import codeql.ruby.frameworks.ActiveStorage
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/**
2+
* Provides modeling for the `ActionMailer` library.
3+
*/
4+
5+
private import codeql.ruby.AST
6+
private import codeql.ruby.ApiGraphs
7+
private import codeql.ruby.frameworks.internal.Rails
8+
9+
/**
10+
* Provides modeling for the `ActionMailer` library.
11+
*/
12+
module ActionMailer {
13+
/**
14+
* A `ClassDeclaration` for a class that extends `ActionMailer::Base`.
15+
* For example,
16+
*
17+
* ```rb
18+
* class FooMailer < ActionMailer::Base
19+
* ...
20+
* end
21+
* ```
22+
*/
23+
class MailerClass extends ClassDeclaration {
24+
MailerClass() {
25+
this.getSuperclassExpr() =
26+
[
27+
API::getTopLevelMember("ActionMailer").getMember("Base"),
28+
// In Rails applications `ApplicationMailer` typically extends
29+
// `ActionMailer::Base`, but we treat it separately in case the
30+
// `ApplicationMailer` definition is not in the database.
31+
API::getTopLevelMember("ApplicationMailer")
32+
].getASubclass().getAValueReachableFromSource().asExpr().getExpr()
33+
}
34+
}
35+
36+
/** A method call with a `self` receiver from within a mailer class */
37+
private class ContextCall extends MethodCall {
38+
private MailerClass mailerClass;
39+
40+
ContextCall() {
41+
this.getReceiver() instanceof SelfVariableAccess and
42+
this.getEnclosingModule() = mailerClass
43+
}
44+
45+
/** Gets the mailer class containing this method. */
46+
MailerClass getMailerClass() { result = mailerClass }
47+
}
48+
49+
/** A call to `params` from within a mailer. */
50+
class ParamsCall extends ContextCall, ParamsCallImpl {
51+
ParamsCall() { this.getMethodName() = "params" }
52+
}
53+
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ paramsCalls
115115
| action_controller/params_flow.rb:144:10:144:15 | call to params |
116116
| action_controller/params_flow.rb:145:32:145:37 | call to params |
117117
| action_controller/params_flow.rb:148:22:148:27 | call to params |
118+
| action_mailer/mailer.rb:3:10:3:15 | call to params |
118119
| active_record/ActiveRecord.rb:28:30:28:35 | call to params |
119120
| active_record/ActiveRecord.rb:29:29:29:34 | call to params |
120121
| active_record/ActiveRecord.rb:30:31:30:36 | call to params |
@@ -189,6 +190,7 @@ paramsSources
189190
| action_controller/params_flow.rb:144:10:144:15 | call to params |
190191
| action_controller/params_flow.rb:145:32:145:37 | call to params |
191192
| action_controller/params_flow.rb:148:22:148:27 | call to params |
193+
| action_mailer/mailer.rb:3:10:3:15 | call to params |
192194
| active_record/ActiveRecord.rb:28:30:28:35 | call to params |
193195
| active_record/ActiveRecord.rb:29:29:29:34 | call to params |
194196
| active_record/ActiveRecord.rb:30:31:30:36 | call to params |
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
class MyMailer < ActionMailer::Base
2+
def foo
3+
sink params[:foo] # $hasTaintFlow
4+
end
5+
end
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
failures
2+
edges
3+
| mailer.rb:3:10:3:15 | call to params : | mailer.rb:3:10:3:21 | ...[...] |
4+
nodes
5+
| mailer.rb:3:10:3:15 | call to params : | semmle.label | call to params : |
6+
| mailer.rb:3:10:3:21 | ...[...] | semmle.label | ...[...] |
7+
subpaths
8+
#select
9+
| mailer.rb:3:10:3:21 | ...[...] | mailer.rb:3:10:3:15 | call to params : | mailer.rb:3:10:3:21 | ...[...] | $@ | mailer.rb:3:10:3:15 | call to params : | call to params : |
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/**
2+
* @kind path-problem
3+
*/
4+
5+
import ruby
6+
import TestUtilities.InlineFlowTest
7+
import PathGraph
8+
import codeql.ruby.frameworks.Rails
9+
10+
class ParamsTaintFlowConf extends DefaultTaintFlowConf {
11+
override predicate isSource(DataFlow::Node n) {
12+
n.asExpr().getExpr() instanceof Rails::ParamsCall
13+
}
14+
}
15+
16+
from DataFlow::PathNode source, DataFlow::PathNode sink, ParamsTaintFlowConf conf
17+
where conf.hasFlowPath(source, sink)
18+
select sink, source, sink, "$@", source, source.toString()

0 commit comments

Comments
 (0)