Merge pull request #10735 from github/nickrolfe/actionmailer

Ruby: add `ActionMailer#params` as a `RemoteFlowSource`
This commit is contained in:
Nick Rolfe
2022-10-12 10:21:11 +01:00
committed by GitHub
7 changed files with 92 additions and 0 deletions

View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Calls to `params` in `ActionMailer` classes are now treated as sources of remote user input.

View File

@@ -5,6 +5,7 @@
private import codeql.ruby.frameworks.Core
private import codeql.ruby.frameworks.ActionCable
private import codeql.ruby.frameworks.ActionController
private import codeql.ruby.frameworks.ActionMailer
private import codeql.ruby.frameworks.ActiveRecord
private import codeql.ruby.frameworks.ActiveResource
private import codeql.ruby.frameworks.ActiveStorage

View File

@@ -0,0 +1,53 @@
/**
* Provides modeling for the `ActionMailer` library.
*/
private import codeql.ruby.AST
private import codeql.ruby.ApiGraphs
private import codeql.ruby.frameworks.internal.Rails
/**
* Provides modeling for the `ActionMailer` library.
*/
module ActionMailer {
/**
* A `ClassDeclaration` for a class that extends `ActionMailer::Base`.
* For example,
*
* ```rb
* class FooMailer < ActionMailer::Base
* ...
* end
* ```
*/
class MailerClass extends ClassDeclaration {
MailerClass() {
this.getSuperclassExpr() =
[
API::getTopLevelMember("ActionMailer").getMember("Base"),
// In Rails applications `ApplicationMailer` typically extends
// `ActionMailer::Base`, but we treat it separately in case the
// `ApplicationMailer` definition is not in the database.
API::getTopLevelMember("ApplicationMailer")
].getASubclass().getAValueReachableFromSource().asExpr().getExpr()
}
}
/** A method call with a `self` receiver from within a mailer class */
private class ContextCall extends MethodCall {
private MailerClass mailerClass;
ContextCall() {
this.getReceiver() instanceof SelfVariableAccess and
this.getEnclosingModule() = mailerClass
}
/** Gets the mailer class containing this method. */
MailerClass getMailerClass() { result = mailerClass }
}
/** A call to `params` from within a mailer. */
class ParamsCall extends ContextCall, ParamsCallImpl {
ParamsCall() { this.getMethodName() = "params" }
}
}

View File

@@ -115,6 +115,7 @@ paramsCalls
| action_controller/params_flow.rb:144:10:144:15 | call to params |
| action_controller/params_flow.rb:145:32:145:37 | call to params |
| action_controller/params_flow.rb:148:22:148:27 | call to params |
| action_mailer/mailer.rb:3:10:3:15 | call to params |
| active_record/ActiveRecord.rb:28:30:28:35 | call to params |
| active_record/ActiveRecord.rb:29:29:29:34 | call to params |
| active_record/ActiveRecord.rb:30:31:30:36 | call to params |
@@ -189,6 +190,7 @@ paramsSources
| action_controller/params_flow.rb:144:10:144:15 | call to params |
| action_controller/params_flow.rb:145:32:145:37 | call to params |
| action_controller/params_flow.rb:148:22:148:27 | call to params |
| action_mailer/mailer.rb:3:10:3:15 | call to params |
| active_record/ActiveRecord.rb:28:30:28:35 | call to params |
| active_record/ActiveRecord.rb:29:29:29:34 | call to params |
| active_record/ActiveRecord.rb:30:31:30:36 | call to params |

View File

@@ -0,0 +1,5 @@
class MyMailer < ActionMailer::Base
def foo
sink params[:foo] # $hasTaintFlow
end
end

View File

@@ -0,0 +1,9 @@
failures
edges
| mailer.rb:3:10:3:15 | call to params : | mailer.rb:3:10:3:21 | ...[...] |
nodes
| mailer.rb:3:10:3:15 | call to params : | semmle.label | call to params : |
| mailer.rb:3:10:3:21 | ...[...] | semmle.label | ...[...] |
subpaths
#select
| 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 : |

View File

@@ -0,0 +1,18 @@
/**
* @kind path-problem
*/
import ruby
import TestUtilities.InlineFlowTest
import PathGraph
import codeql.ruby.frameworks.Rails
class ParamsTaintFlowConf extends DefaultTaintFlowConf {
override predicate isSource(DataFlow::Node n) {
n.asExpr().getExpr() instanceof Rails::ParamsCall
}
}
from DataFlow::PathNode source, DataFlow::PathNode sink, ParamsTaintFlowConf conf
where conf.hasFlowPath(source, sink)
select sink, source, sink, "$@", source, source.toString()