Skip to content

Commit 2c63dba

Browse files
authored
Merge pull request github#11954 from hmac/sinatra
Ruby: Model Sinatra
2 parents ebab6ec + 3734a54 commit 2c63dba

File tree

10 files changed

+609
-0
lines changed

10 files changed

+609
-0
lines changed
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Accesses of `params` in Sinatra applications are now recognised as HTTP input accesses.
5+
* Data flow is tracked from Sinatra route handlers to ERB files.
6+
* Data flow is tracked between basic Sinatra filters (those without URL patterns) and their corresponding route handlers.

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,4 +27,5 @@ private import codeql.ruby.frameworks.ActionDispatch
2727
private import codeql.ruby.frameworks.PosixSpawn
2828
private import codeql.ruby.frameworks.StringFormatters
2929
private import codeql.ruby.frameworks.Json
30+
private import codeql.ruby.frameworks.Sinatra
3031
private import codeql.ruby.frameworks.Twirp

ruby/ql/lib/codeql/ruby/dataflow/SSA.qll

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,24 @@ module Ssa {
294294
override Location getLocation() { result = this.getBasicBlock().getLocation() }
295295
}
296296

297+
/**
298+
* An SSA definition inserted at the beginning of a scope to represent a captured `self` variable.
299+
* For example, in
300+
*
301+
* ```rb
302+
* def m(x)
303+
* x.tap do |x|
304+
* foo(x)
305+
* end
306+
* end
307+
* ```
308+
*
309+
* an entry definition for `self` is inserted at the start of the `do` block.
310+
*/
311+
class CapturedSelfDefinition extends CapturedEntryDefinition {
312+
CapturedSelfDefinition() { this.getSourceVariable() instanceof SelfVariable }
313+
}
314+
297315
/**
298316
* An SSA definition inserted at a call that may update the value of a captured
299317
* variable. For example, in
Lines changed: 300 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,300 @@
1+
/** Provides modeling for the Sinatra library. */
2+
3+
private import codeql.ruby.controlflow.CfgNodes::ExprNodes
4+
private import codeql.ruby.DataFlow
5+
private import codeql.ruby.Concepts
6+
private import codeql.ruby.AST
7+
private import codeql.ruby.dataflow.FlowSummary
8+
private import codeql.ruby.dataflow.internal.DataFlowPrivate as DataFlowPrivate
9+
private import codeql.ruby.dataflow.SSA
10+
11+
/** Provides modeling for the Sinatra library. */
12+
module Sinatra {
13+
/**
14+
* A Sinatra application.
15+
*
16+
* ```rb
17+
* class MyApp < Sinatra::Base
18+
* get "/" do
19+
* erb :home
20+
* end
21+
* end
22+
* ```
23+
*/
24+
class App extends DataFlow::ClassNode {
25+
App() { this = DataFlow::getConstant("Sinatra").getConstant("Base").getADescendentModule() }
26+
27+
/**
28+
* Gets a route defined in this application.
29+
*/
30+
Route getARoute() { result.getApp() = this }
31+
}
32+
33+
/**
34+
* A Sinatra route handler. HTTP requests with a matching method and path will
35+
* be handled by the block. For example, the following route will handle `GET`
36+
* requests with path `/`.
37+
*
38+
* ```rb
39+
* get "/" do
40+
* erb :home
41+
* end
42+
* ```
43+
*/
44+
class Route extends DataFlow::CallNode {
45+
private App app;
46+
47+
Route() {
48+
this =
49+
app.getAModuleLevelCall([
50+
"get", "post", "put", "patch", "delete", "options", "link", "unlink"
51+
])
52+
}
53+
54+
/**
55+
* Gets the application that defines this route.
56+
*/
57+
App getApp() { result = app }
58+
59+
/**
60+
* Gets the body of this route.
61+
*/
62+
DataFlow::BlockNode getBody() { result = this.getBlock() }
63+
}
64+
65+
/**
66+
* An access to the parameters of an HTTP request in a Sinatra route handler or callback.
67+
*/
68+
private class Params extends DataFlow::CallNode, Http::Server::RequestInputAccess::Range {
69+
Params() {
70+
this.asExpr().getExpr().getEnclosingCallable() =
71+
[any(Route r).getBody(), any(Filter f).getBody()].asCallableAstNode() and
72+
this.getMethodName() = "params"
73+
}
74+
75+
override string getSourceType() { result = "Sinatra::Base#params" }
76+
77+
override Http::Server::RequestInputKind getKind() {
78+
result = Http::Server::parameterInputKind()
79+
}
80+
}
81+
82+
/**
83+
* A call which renders an ERB template as an HTTP response.
84+
*/
85+
class ErbCall extends DataFlow::CallNode {
86+
private Route route;
87+
88+
ErbCall() {
89+
this.asExpr().getExpr().getEnclosingCallable() = route.getBody().asCallableAstNode() and
90+
this.getMethodName() = "erb"
91+
}
92+
93+
/**
94+
* Gets the template file corresponding to this call.
95+
*/
96+
ErbFile getTemplateFile() { result = getTemplateFile(this.asExpr().getExpr()) }
97+
98+
/**
99+
* Gets the route containing this call.
100+
*/
101+
Route getRoute() { result = route }
102+
}
103+
104+
/**
105+
* Gets the template file referred to by `erbCall`.
106+
* This works on the AST level to avoid non-monotonic reecursion in `ErbLocalsHashSyntheticGlobal`.
107+
*/
108+
private ErbFile getTemplateFile(MethodCall erbCall) {
109+
erbCall.getMethodName() = "erb" and
110+
result.getTemplateName() = erbCall.getArgument(0).getConstantValue().getStringlikeValue() and
111+
result.getRelativePath().matches("%views/%")
112+
}
113+
114+
/**
115+
* Like `Location.toString`, but displays the relative path rather than the full path.
116+
*/
117+
private string locationRelativePathToString(Location loc) {
118+
result =
119+
loc.getFile().getRelativePath() + "@" + loc.getStartLine() + ":" + loc.getStartColumn() + ":" +
120+
loc.getEndLine() + ":" + loc.getEndColumn()
121+
}
122+
123+
/**
124+
* A synthetic global representing the hash of local variables passed to an ERB template.
125+
*/
126+
class ErbLocalsHashSyntheticGlobal extends SummaryComponent::SyntheticGlobal {
127+
private string id;
128+
private MethodCall erbCall;
129+
private ErbFile erbFile;
130+
131+
ErbLocalsHashSyntheticGlobal() {
132+
this = "SinatraErbLocalsHash(" + id + ")" and
133+
id = erbFile.getRelativePath() + "," + locationRelativePathToString(erbCall.getLocation()) and
134+
erbCall.getMethodName() = "erb" and
135+
erbFile = getTemplateFile(erbCall)
136+
}
137+
138+
/**
139+
* Gets the `erb` call associated with this global.
140+
*/
141+
MethodCall getErbCall() { result = erbCall }
142+
143+
/**
144+
* Gets the ERB template that this global contains the locals for.
145+
*/
146+
ErbFile getErbFile() { result = erbFile }
147+
148+
/**
149+
* Gets a unique identifer for this global.
150+
*/
151+
string getId() { result = id }
152+
}
153+
154+
/**
155+
* A summary for `Sinatra::Base#erb`. This models the first half of the flow
156+
* from the `locals` keyword argument to variables in the ERB template. The
157+
* second half is modeled by `ErbLocalsAccessSummary`.
158+
*/
159+
private class ErbLocalsSummary extends SummarizedCallable {
160+
ErbLocalsSummary() { this = "Sinatra::Base#erb" }
161+
162+
override MethodCall getACall() { result = any(ErbCall c).asExpr().getExpr() }
163+
164+
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
165+
input = "Argument[locals:]" and
166+
output = "SyntheticGlobal[" + any(ErbLocalsHashSyntheticGlobal global) + "]" and
167+
preservesValue = true
168+
}
169+
}
170+
171+
/**
172+
* A summary for accessing a local variable in an ERB template.
173+
* This is the second half of the modeling of the flow from the `locals`
174+
* keyword argument to variables in the ERB template.
175+
* The first half is modeled by `ErbLocalsSummary`.
176+
*/
177+
private class ErbLocalsAccessSummary extends SummarizedCallable {
178+
private ErbLocalsHashSyntheticGlobal global;
179+
private string local;
180+
181+
ErbLocalsAccessSummary() {
182+
this = "sinatra_erb_locals_access()" + global.getId() + "#" + local and
183+
local = any(MethodCall c | c.getLocation().getFile() = global.getErbFile()).getMethodName() and
184+
local = any(Pair p).getKey().getConstantValue().getStringlikeValue()
185+
}
186+
187+
override MethodCall getACall() {
188+
result.getLocation().getFile() = global.getErbFile() and
189+
result.getMethodName() = local and
190+
result.getReceiver() instanceof SelfVariableReadAccess
191+
}
192+
193+
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
194+
input = "SyntheticGlobal[" + global + "].Element[:" + local + "]" and
195+
output = "ReturnValue" and
196+
preservesValue = true
197+
}
198+
}
199+
200+
/**
201+
* A class representing Sinatra filters AKA callbacks.
202+
*
203+
* Filters are run before or after the route handler. They can modify the
204+
* request and response, and share instance variables with the route handler.
205+
*/
206+
class Filter extends DataFlow::CallNode {
207+
private App app;
208+
209+
Filter() { this = app.getAModuleLevelCall(["before", "after"]) }
210+
211+
/** Gets the app which this filter belongs to. */
212+
App getApp() { result = app }
213+
214+
/**
215+
* Gets the pattern which constrains this route, if any. In the example below, the pattern is `/protected/*`.
216+
* Patterns are typically given as strings, and are interpreted by the `mustermann` gem (they are not regular expressions).
217+
* ```rb
218+
* before '/protected/*' do
219+
* authenticate!
220+
* end
221+
* ```
222+
*/
223+
DataFlow::ExprNode getPattern() { result = this.getArgument(0) }
224+
225+
/**
226+
* Holds if this filter has a pattern.
227+
*/
228+
predicate hasPattern() { exists(this.getPattern()) }
229+
230+
/**
231+
* Gets the body of this filter.
232+
*/
233+
DataFlow::BlockNode getBody() { result = this.getBlock() }
234+
}
235+
236+
/**
237+
* A class for Sinatra `before` filters. These run before the route handler.
238+
*/
239+
class BeforeFilter extends Filter {
240+
BeforeFilter() { this.getMethodName() = "before" }
241+
}
242+
243+
/**
244+
* A class for Sinatra `after` filters. These run after the route handler.
245+
*/
246+
class AfterFilter extends Filter {
247+
AfterFilter() { this.getMethodName() = "after" }
248+
}
249+
250+
/**
251+
* A class defining additional jump steps arising from filters.
252+
* This only models flow between filters with no patterns - i.e. those that apply to all routes.
253+
* Filters with patterns are not yet modeled.
254+
*/
255+
class FilterJumpStep extends DataFlowPrivate::AdditionalJumpStep {
256+
/**
257+
* Holds if data can flow from `pred` to `succ` via a callback chain.
258+
*/
259+
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
260+
exists(BeforeFilter filter, Route route |
261+
// the filter and route belong to the same app
262+
filter.getApp() = route.getApp() and
263+
// the filter applies to all routes
264+
not filter.hasPattern() and
265+
selfPostUpdate(pred, filter.getApp(), filter.getBody().asExpr().getExpr()) and
266+
blockCapturedSelfParameterNode(succ, route.getBody().asExpr().getExpr())
267+
)
268+
}
269+
}
270+
271+
/**
272+
* Holds if `n` is a post-update node for the `self` parameter of `app` in block `b`.
273+
*
274+
* In this example, `n` is the post-update node for `@foo = 1`.
275+
* ```rb
276+
* class MyApp < Sinatra::Base
277+
* before do
278+
* @foo = 1
279+
* end
280+
* end
281+
* ```
282+
*/
283+
private predicate selfPostUpdate(DataFlow::PostUpdateNode n, App app, Block b) {
284+
n.getPreUpdateNode().asExpr().getExpr() =
285+
any(SelfVariableAccess self |
286+
pragma[only_bind_into](b) = self.getEnclosingCallable() and
287+
self.getVariable().getDeclaringScope() = app.getADeclaration()
288+
)
289+
}
290+
291+
/**
292+
* Holds if `n` is a node representing the `self` parameter captured by block `b`.
293+
*/
294+
private predicate blockCapturedSelfParameterNode(DataFlow::Node n, Block b) {
295+
exists(Ssa::CapturedSelfDefinition d |
296+
n.(DataFlowPrivate::SsaDefinitionExtNode).getDefinitionExt() = d and
297+
d.getBasicBlock().getScope() = b
298+
)
299+
}
300+
}
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
failures
2+
| views/index.erb:2:10:2:12 | call to foo | Unexpected result: hasTaintFlow= |
3+
edges
4+
| app.rb:75:5:75:8 | [post] self [@foo] : | app.rb:76:32:76:35 | self [@foo] : |
5+
| app.rb:75:12:75:17 | call to params : | app.rb:75:12:75:24 | ...[...] : |
6+
| app.rb:75:12:75:24 | ...[...] : | app.rb:75:5:75:8 | [post] self [@foo] : |
7+
| app.rb:76:32:76:35 | @foo : | views/index.erb:2:10:2:12 | call to foo |
8+
| app.rb:76:32:76:35 | self [@foo] : | app.rb:76:32:76:35 | @foo : |
9+
| app.rb:95:10:95:14 | self [@user] : | app.rb:95:10:95:14 | @user |
10+
| app.rb:103:5:103:9 | [post] self [@user] : | app.rb:95:10:95:14 | self [@user] : |
11+
| app.rb:103:13:103:22 | call to source : | app.rb:103:5:103:9 | [post] self [@user] : |
12+
nodes
13+
| app.rb:75:5:75:8 | [post] self [@foo] : | semmle.label | [post] self [@foo] : |
14+
| app.rb:75:12:75:17 | call to params : | semmle.label | call to params : |
15+
| app.rb:75:12:75:24 | ...[...] : | semmle.label | ...[...] : |
16+
| app.rb:76:32:76:35 | @foo : | semmle.label | @foo : |
17+
| app.rb:76:32:76:35 | self [@foo] : | semmle.label | self [@foo] : |
18+
| app.rb:95:10:95:14 | @user | semmle.label | @user |
19+
| app.rb:95:10:95:14 | self [@user] : | semmle.label | self [@user] : |
20+
| app.rb:103:5:103:9 | [post] self [@user] : | semmle.label | [post] self [@user] : |
21+
| app.rb:103:13:103:22 | call to source : | semmle.label | call to source : |
22+
| views/index.erb:2:10:2:12 | call to foo | semmle.label | call to foo |
23+
subpaths
24+
#select
25+
| views/index.erb:2:10:2:12 | call to foo | app.rb:75:12:75:17 | call to params : | views/index.erb:2:10:2:12 | call to foo | $@ | app.rb:75:12:75:17 | call to params : | call to params : |
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/**
2+
* @kind path-problem
3+
*/
4+
5+
import ruby
6+
import TestUtilities.InlineFlowTest
7+
import PathGraph
8+
import codeql.ruby.frameworks.Sinatra
9+
import codeql.ruby.Concepts
10+
11+
class SinatraConf extends DefaultTaintFlowConf {
12+
override predicate isSource(DataFlow::Node source) {
13+
source instanceof Http::Server::RequestInputAccess::Range
14+
}
15+
}
16+
17+
from DataFlow::PathNode source, DataFlow::PathNode sink, SinatraConf conf
18+
where conf.hasFlowPath(source, sink)
19+
select sink, source, sink, "$@", source, source.toString()

0 commit comments

Comments
 (0)