Skip to content

Commit 055641e

Browse files
alexrfordalexrford
authored
Merge pull request #7062 from github/ruby/rails-csrf
Ruby: Add `rb/csrf-protection-disabled` query
2 parents 8cccee6 + 68c3c16 commit 055641e

File tree

18 files changed

+337
-0
lines changed

18 files changed

+337
-0
lines changed

‎ruby/change-notes/2021-11-04-csrf-protection-disabled.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
lgtm,codescanning
2+
* A new query (`rb/csrf-protection-disabled`) has been added. The query finds cases where cross-site forgery protection is explictly disabled.

‎ruby/ql/lib/codeql/ruby/Concepts.qll

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -596,6 +596,37 @@ module OrmInstantiation {
596596
}
597597
}
598598

599+
/**
600+
* A data-flow node that may set or unset Cross-site request forgery protection.
601+
*
602+
* Extend this class to refine existing API models. If you want to model new APIs,
603+
* extend `CSRFProtectionSetting::Range` instead.
604+
*/
605+
class CSRFProtectionSetting extends DataFlow::Node instanceof CSRFProtectionSetting::Range {
606+
/**
607+
* Gets the boolean value corresponding to if CSRF protection is enabled
608+
* (`true`) or disabled (`false`) by this node.
609+
*/
610+
boolean getVerificationSetting() { result = super.getVerificationSetting() }
611+
}
612+
613+
/** Provides a class for modeling new CSRF protection setting APIs. */
614+
module CSRFProtectionSetting {
615+
/**
616+
* A data-flow node that may set or unset Cross-site request forgery protection.
617+
*
618+
* Extend this class to model new APIs. If you want to refine existing API models,
619+
* extend `CSRFProtectionSetting` instead.
620+
*/
621+
abstract class Range extends DataFlow::Node {
622+
/**
623+
* Gets the boolean value corresponding to if CSRF protection is enabled
624+
* (`true`) or disabled (`false`) by this node.
625+
*/
626+
abstract boolean getVerificationSetting();
627+
}
628+
}
629+
599630
/** Provides classes for modeling path-related APIs. */
600631
module Path {
601632
/**

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ private import codeql.ruby.frameworks.ActionController
66
private import codeql.ruby.frameworks.ActiveRecord
77
private import codeql.ruby.frameworks.ActiveStorage
88
private import codeql.ruby.frameworks.ActionView
9+
private import codeql.ruby.frameworks.Rails
910
private import codeql.ruby.frameworks.StandardLibrary
1011
private import codeql.ruby.frameworks.Files
1112
private import codeql.ruby.frameworks.HttpClients

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,3 +257,21 @@ predicate controllerTemplateFile(ActionControllerControllerClass cls, ErbFile te
257257
)
258258
)
259259
}
260+
261+
/**
262+
* A call to either `skip_forgery_protection` or
263+
* `skip_before_action :verify_authenticity_token` to disable CSRF authenticity
264+
* token protection.
265+
*/
266+
class ActionControllerSkipForgeryProtectionCall extends CSRFProtectionSetting::Range {
267+
ActionControllerSkipForgeryProtectionCall() {
268+
exists(MethodCall call | call = this.asExpr().getExpr() |
269+
call.getMethodName() = "skip_forgery_protection"
270+
or
271+
call.getMethodName() = "skip_before_action" and
272+
call.getAnArgument().(SymbolLiteral).getValueText() = "verify_authenticity_token"
273+
)
274+
}
275+
276+
override boolean getVerificationSetting() { result = false }
277+
}

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

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
/**
2+
* Provides classes for working with Rails.
3+
*/
4+
5+
private import codeql.files.FileSystem
6+
private import codeql.ruby.AST
7+
private import codeql.ruby.Concepts
8+
private import codeql.ruby.DataFlow
9+
private import codeql.ruby.frameworks.ActionController
10+
private import codeql.ruby.frameworks.ActionView
11+
private import codeql.ruby.frameworks.ActiveRecord
12+
private import codeql.ruby.frameworks.ActiveStorage
13+
private import codeql.ruby.ast.internal.Module
14+
private import codeql.ruby.ApiGraphs
15+
16+
/**
17+
* A reference to either `Rails::Railtie`, `Rails::Engine`, or `Rails::Application`.
18+
* `Engine` and `Application` extend `Railtie`, but may not have definitions present in the database.
19+
*/
20+
private class RailtieClassAccess extends ConstantReadAccess {
21+
RailtieClassAccess() {
22+
this.getScopeExpr().(ConstantAccess).getName() = "Rails" and
23+
this.getName() = ["Railtie", "Engine", "Application"]
24+
}
25+
}
26+
27+
// A `ClassDeclaration` that (transitively) extends `Rails::Railtie`
28+
private class RailtieClass extends ClassDeclaration {
29+
RailtieClass() {
30+
this.getSuperclassExpr() instanceof RailtieClassAccess or
31+
exists(RailtieClass other | other.getModule() = resolveScopeExpr(this.getSuperclassExpr()))
32+
}
33+
}
34+
35+
private DataFlow::CallNode getAConfigureCallNode() {
36+
// `Rails.application.configure`
37+
result = API::getTopLevelMember("Rails").getReturn("application").getAMethodCall("configure")
38+
or
39+
// `Rails::Application.configure`
40+
exists(ConstantReadAccess read, RailtieClass cls |
41+
read = result.getReceiver().asExpr().getExpr() and
42+
resolveScopeExpr(read) = cls.getModule() and
43+
result.asExpr().getExpr().(MethodCall).getMethodName() = "configure"
44+
)
45+
}
46+
47+
/**
48+
* An access to a Rails config object.
49+
*/
50+
private class ConfigSourceNode extends DataFlow::LocalSourceNode {
51+
ConfigSourceNode() {
52+
// `Foo < Rails::Application ... config ...`
53+
exists(MethodCall configCall | this.asExpr().getExpr() = configCall |
54+
configCall.getMethodName() = "config" and
55+
configCall.getEnclosingModule() instanceof RailtieClass
56+
)
57+
or
58+
// `Rails.application.config`
59+
this =
60+
API::getTopLevelMember("Rails")
61+
.getReturn("application")
62+
.getReturn("config")
63+
.getAnImmediateUse()
64+
or
65+
// `Rails.application.configure { ... config ... }`
66+
// `Rails::Application.configure { ... config ... }`
67+
exists(DataFlow::CallNode configureCallNode, Block block, MethodCall configCall |
68+
configCall = this.asExpr().getExpr()
69+
|
70+
configureCallNode = getAConfigureCallNode() and
71+
block = configureCallNode.asExpr().getExpr().(MethodCall).getBlock() and
72+
configCall.getParent+() = block and
73+
configCall.getMethodName() = "config"
74+
)
75+
}
76+
}
77+
78+
private class ConfigNode extends DataFlow::Node {
79+
ConfigNode() { exists(ConfigSourceNode src | src.flowsTo(this)) }
80+
}
81+
82+
// A call where the Rails application config is the receiver
83+
private class CallAgainstConfig extends DataFlow::CallNode {
84+
CallAgainstConfig() { this.getReceiver() instanceof ConfigNode }
85+
86+
MethodCall getCall() { result = this.asExpr().getExpr() }
87+
88+
Block getBlock() { result = this.getCall().getBlock() }
89+
}
90+
91+
private class ActionControllerConfigNode extends DataFlow::Node {
92+
ActionControllerConfigNode() {
93+
exists(CallAgainstConfig source | source.getCall().getMethodName() = "action_controller" |
94+
source.flowsTo(this)
95+
)
96+
}
97+
}
98+
99+
/** Holds if `node` can contain `value`. */
100+
private predicate hasBooleanValue(DataFlow::Node node, boolean value) {
101+
exists(DataFlow::LocalSourceNode literal |
102+
literal.asExpr().getExpr().(BooleanLiteral).getValue() = value and
103+
literal.flowsTo(node)
104+
)
105+
}
106+
107+
// `<actionControllerConfig>.allow_forgery_protection = <verificationSetting>`
108+
private DataFlow::CallNode getAnAllowForgeryProtectionCall(boolean verificationSetting) {
109+
// exclude some test configuration
110+
not (
111+
result.getLocation().getFile().getRelativePath().matches("%test/%") or
112+
result.getLocation().getFile().getStem() = "test"
113+
) and
114+
result.getReceiver() instanceof ActionControllerConfigNode and
115+
result.asExpr().getExpr().(MethodCall).getMethodName() = "allow_forgery_protection=" and
116+
hasBooleanValue(result.getArgument(0), verificationSetting)
117+
}
118+
119+
/**
120+
* A `DataFlow::Node` that may enable or disable Rails CSRF protection in
121+
* production code.
122+
*/
123+
private class AllowForgeryProtectionSetting extends CSRFProtectionSetting::Range {
124+
private boolean verificationSetting;
125+
126+
AllowForgeryProtectionSetting() { this = getAnAllowForgeryProtectionCall(verificationSetting) }
127+
128+
override boolean getVerificationSetting() { result = verificationSetting }
129+
}
130+
// TODO: initialization hooks, e.g. before_configuration, after_initialize...
131+
// TODO: initializers

‎ruby/ql/src/queries/security/cwe-352/CSRFProtectionDisabled.qhelp

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
Cross-site request forgery (CSRF) is a type of vulnerability in which an
9+
attacker is able to force a user carry out an action that the user did
10+
not intend.
11+
</p>
12+
13+
<p>
14+
The attacker tricks an authenticated user into submitting a request to the
15+
web application. Typically this request will result in a state change on
16+
the server, such as changing the user's password. The request can be
17+
initiated when the user visits a site controlled by the attacker. If the
18+
web application relies only on cookies for authentication, or on other
19+
credentials that are automatically included in the request, then this
20+
request will appear as legitimate to the server.
21+
</p>
22+
23+
<p>
24+
A common countermeasure for CSRF is to generate a unique token to be
25+
included in the HTML sent from the server to a user. This token can be
26+
used as a hidden field to be sent back with requests to the server, where
27+
the server can then check that the token is valid and associated with the
28+
relevant user session.
29+
</p>
30+
</overview>
31+
32+
<recommendation>
33+
<p>
34+
In many web frameworks, CSRF protection is enabled by default. In these
35+
cases, using the default configuration is sufficient to guard against most
36+
CSRF attacks.
37+
</p>
38+
</recommendation>
39+
40+
<example>
41+
<p>
42+
The following example shows a case where CSRF protection is disabled by
43+
skipping token verification.
44+
</p>
45+
46+
<sample src="examples/UsersController.rb"/>
47+
48+
<p>
49+
Verification can be re-enabled by removing the call to
50+
<code>skip_before_action</code>.
51+
</p>
52+
53+
</example>
54+
55+
<references>
56+
<li>Wikipedia: <a href="https://en.wikipedia.org/wiki/Cross-site_request_forgery">Cross-site request forgery</a></li>
57+
<li>OWASP: <a href="https://owasp.org/www-community/attacks/csrf">Cross-site request forgery</a></li>
58+
<li>Securing Rails Applications: <a href="https://guides.rubyonrails.org/security.html#cross-site-request-forgery-csrf">Cross-Site Request Forgery (CSRF)</a></li>
59+
</references>
60+
61+
</qhelp>

‎ruby/ql/src/queries/security/cwe-352/CSRFProtectionDisabled.ql

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/**
2+
* @name CSRF protection disabled
3+
* @description Disabling CSRF protection makes the application vulnerable to
4+
* a Cross-Site Request Forgery (CSRF) attack.
5+
* @kind problem
6+
* @problem.severity warning
7+
* @security-severity 8.8
8+
* @precision high
9+
* @id rb/csrf-protection-disabled
10+
* @tags security
11+
* external/cwe/cwe-352
12+
*/
13+
14+
import ruby
15+
import codeql.ruby.Concepts
16+
17+
from CSRFProtectionSetting s
18+
where s.getVerificationSetting() = false
19+
select s, "Potential CSRF vulnerability due to forgery protection being disabled."

‎ruby/ql/src/queries/security/cwe-352/examples/UsersController.rb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
class UsersController < ApplicationController
2+
skip_before_action :verify_authenticity_token
3+
end

‎ruby/ql/test/query-tests/security/cwe-352/CSRFProtectionDisabled.expected

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
| railsapp/app/controllers/users_controller.rb:4:3:4:47 | call to skip_before_action | Potential CSRF vulnerability due to forgery protection being disabled. |
2+
| railsapp/config/application.rb:15:5:15:53 | call to allow_forgery_protection= | Potential CSRF vulnerability due to forgery protection being disabled. |
3+
| railsapp/config/environments/development.rb:5:3:5:51 | call to allow_forgery_protection= | Potential CSRF vulnerability due to forgery protection being disabled. |
4+
| railsapp/config/environments/production.rb:5:3:5:51 | call to allow_forgery_protection= | Potential CSRF vulnerability due to forgery protection being disabled. |

‎ruby/ql/test/query-tests/security/cwe-352/CSRFProtectionDisabled.qlref

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
queries/security/cwe-352/CSRFProtectionDisabled.ql

0 commit comments

Comments
 (0)