Skip to content

Commit 5ce6e63

Browse files
alexrfordalexrford
committed
Ruby: Tidy Rails.qll to make adding new settings modeling easier
1 parent 737f733 commit 5ce6e63

File tree

1 file changed

+120
-63
lines changed

1 file changed

+120
-63
lines changed

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

Lines changed: 120 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ private import codeql.ruby.frameworks.ActiveRecord
1212
private import codeql.ruby.frameworks.ActiveStorage
1313
private import codeql.ruby.ast.internal.Module
1414
private import codeql.ruby.ApiGraphs
15+
private import codeql.ruby.security.CryptoAlgorithms
1516

1617
/**
1718
* A reference to either `Rails::Railtie`, `Rails::Engine`, or `Rails::Application`.
@@ -45,87 +46,143 @@ private DataFlow::CallNode getAConfigureCallNode() {
4546
}
4647

4748
/**
48-
* An access to a Rails config object.
49+
* Classes representing accesses to the Rails config object.
4950
*/
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-
)
51+
private module Config {
52+
/**
53+
* An access to a Rails config object.
54+
*/
55+
private class SourceNode extends DataFlow::LocalSourceNode {
56+
SourceNode() {
57+
// `Foo < Rails::Application ... config ...`
58+
exists(MethodCall configCall | this.asExpr().getExpr() = configCall |
59+
configCall.getMethodName() = "config" and
60+
configCall.getEnclosingModule() instanceof RailtieClass
61+
)
62+
or
63+
// `Rails.application.config`
64+
this =
65+
API::getTopLevelMember("Rails")
66+
.getReturn("application")
67+
.getReturn("config")
68+
.getAnImmediateUse()
69+
or
70+
// `Rails.application.configure { ... config ... }`
71+
// `Rails::Application.configure { ... config ... }`
72+
exists(DataFlow::CallNode configureCallNode, Block block, MethodCall configCall |
73+
configCall = this.asExpr().getExpr()
74+
|
75+
configureCallNode = getAConfigureCallNode() and
76+
block = configureCallNode.asExpr().getExpr().(MethodCall).getBlock() and
77+
configCall.getParent+() = block and
78+
configCall.getMethodName() = "config"
79+
)
80+
}
7581
}
76-
}
7782

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 }
83+
/**
84+
* A reference to the Rails config object.
85+
*/
86+
class Node extends DataFlow::Node {
87+
Node() { exists(SourceNode src | src.flowsTo(this)) }
88+
}
8589

86-
MethodCall getCall() { result = this.asExpr().getExpr() }
90+
/**
91+
* A reference to the ActionController config object.
92+
*/
93+
class ActionControllerNode extends DataFlow::Node {
94+
ActionControllerNode() {
95+
exists(DataFlow::CallNode source |
96+
source.getReceiver() instanceof Node and
97+
source.getMethodName() = "action_controller"
98+
|
99+
source.flowsTo(this)
100+
)
101+
}
102+
}
87103

88-
Block getBlock() { result = this.getCall().getBlock() }
104+
/**
105+
* A reference to the ActionDispatch config object.
106+
*/
107+
class ActionDispatchNode extends DataFlow::Node {
108+
ActionDispatchNode() {
109+
exists(DataFlow::CallNode source |
110+
source.getReceiver() instanceof Node and
111+
source.getMethodName() = "action_dispatch"
112+
|
113+
source.flowsTo(this)
114+
)
115+
}
116+
}
89117
}
90118

91-
private class ActionControllerConfigNode extends DataFlow::Node {
92-
ActionControllerConfigNode() {
93-
exists(CallAgainstConfig source | source.getCall().getMethodName() = "action_controller" |
94-
source.flowsTo(this)
119+
/**
120+
* Classes representing nodes that set a Rails configuration value.
121+
*/
122+
private module Settings {
123+
private predicate isInTestConfiguration(Location loc) {
124+
loc.getFile().getRelativePath().matches("%test/%") or
125+
loc.getFile().getStem() = "test"
126+
}
127+
128+
private DataFlow::Node getTransitiveReceiver(DataFlow::CallNode c) {
129+
exists(DataFlow::Node recv |
130+
recv = c.getReceiver() and
131+
(
132+
result = recv
133+
or
134+
recv instanceof DataFlow::CallNode and
135+
result = getTransitiveReceiver(recv)
136+
)
95137
)
96138
}
97-
}
98139

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-
}
140+
private class Setting extends DataFlow::CallNode {
141+
Setting() {
142+
// exclude some test configuration
143+
not isInTestConfiguration(this.getLocation()) and
144+
getTransitiveReceiver(this) instanceof Config::Node
145+
}
146+
}
147+
148+
private class LiteralSetting extends Setting {
149+
Literal valueLiteral;
106150

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)
151+
LiteralSetting() {
152+
exists(DataFlow::LocalSourceNode lsn |
153+
lsn.asExpr().getExpr() = valueLiteral and
154+
lsn.flowsTo(this.getArgument(0))
155+
)
156+
}
157+
158+
string getValueText() { result = valueLiteral.getValueText() }
159+
160+
string getSettingString() { result = this.getMethodName() + this.getValueText() }
161+
}
162+
163+
/**
164+
* A node that sets a boolean value.
165+
*/
166+
class BooleanSetting extends LiteralSetting {
167+
override BooleanLiteral valueLiteral;
168+
169+
boolean getValue() { result = valueLiteral.getValue() }
170+
}
117171
}
118172

119173
/**
120174
* A `DataFlow::Node` that may enable or disable Rails CSRF protection in
121175
* production code.
122176
*/
123-
private class AllowForgeryProtectionSetting extends CSRFProtectionSetting::Range {
124-
private boolean verificationSetting;
125-
126-
AllowForgeryProtectionSetting() { this = getAnAllowForgeryProtectionCall(verificationSetting) }
177+
private class AllowForgeryProtectionSetting extends Settings::BooleanSetting,
178+
CSRFProtectionSetting::Range {
179+
AllowForgeryProtectionSetting() {
180+
this.getReceiver() instanceof Config::ActionControllerNode and
181+
this.getMethodName() = "allow_forgery_protection="
182+
}
127183

128-
override boolean getVerificationSetting() { result = verificationSetting }
184+
override boolean getVerificationSetting() { result = this.getValue() }
129185
}
186+
130187
// TODO: initialization hooks, e.g. before_configuration, after_initialize...
131188
// TODO: initializers

0 commit comments

Comments
 (0)