diff --git a/ruby/ql/lib/codeql/ruby/frameworks/Sinatra.qll b/ruby/ql/lib/codeql/ruby/frameworks/Sinatra.qll index d83ac2f17df..7d593abef42 100644 --- a/ruby/ql/lib/codeql/ruby/frameworks/Sinatra.qll +++ b/ruby/ql/lib/codeql/ruby/frameworks/Sinatra.qll @@ -10,12 +10,37 @@ private import codeql.ruby.dataflow.SSA /** Provides modeling for the Sinatra library. */ module Sinatra { + /** + * A Sinatra application. + * + * ```rb + * class MyApp < Sinatra::Base + * get "/" do + * erb :home + * end + * end + * ``` + */ class App extends DataFlow::ClassNode { App() { this = DataFlow::getConstant("Sinatra").getConstant("Base").getADescendentModule() } + /** + * Gets a route defined in this application. + */ Route getARoute() { result.getApp() = this } } + /** + * A Sinatra route handler. HTTP requests with a matching method and path will + * be handled by the block. For example, the following route will handle `GET` + * requests with path `/`. + * + * ```rb + * get "/" do + * erb :home + * end + * ``` + */ class Route extends DataFlow::CallNode { private App app; @@ -26,11 +51,20 @@ module Sinatra { ]) } + /** + * Gets the application that defines this route. + */ App getApp() { result = app } + /** + * Gets the body of this route. + */ DataFlow::BlockNode getBody() { result = this.getBlock() } } + /** + * An access to the parameters of an HTTP request in a Sinatra route handler or callback. + */ private class Params extends DataFlow::CallNode, Http::Server::RequestInputAccess::Range { Params() { this.asExpr().getExpr().getEnclosingCallable() = @@ -45,6 +79,9 @@ module Sinatra { } } + /** + * A call which renders an ERB template as an HTTP response. + */ class ErbCall extends DataFlow::CallNode { private Route route; @@ -57,6 +94,11 @@ module Sinatra { * Gets the template file corresponding to this call. */ ErbFile getTemplateFile() { result = getTemplateFile(this.asExpr().getExpr()) } + + /** + * Gets the route containing this call. + */ + Route getRoute() { result = route } } /** @@ -78,6 +120,9 @@ module Sinatra { loc.getEndLine() + ":" + loc.getEndColumn() } + /** + * A synthetic global representing the hash of local variables passed to an ERB template. + */ class ErbLocalsHashSyntheticGlobal extends SummaryComponent::SyntheticGlobal { private string id; private MethodCall erbCall; @@ -90,25 +135,44 @@ module Sinatra { erbFile = getTemplateFile(erbCall) } + /** + * Gets the `erb` call associated with this global. + */ + MethodCall getErbCall() { result = erbCall } + + /** + * Gets the ERB template that this global contains the locals for. + */ ErbFile getErbFile() { result = erbFile } + /** + * Gets a unique identifer for this global. + */ string getId() { result = id } } + /** + * A summary for `Sinatra::Base#erb`. This models the first half of the flow + * from the `locals` keyword argument to variables in the ERB template. The + * second half is modeled by `ErbLocalsAccessSummary`. + */ private class ErbLocalsSummary extends SummarizedCallable { - private ErbLocalsHashSyntheticGlobal global; - ErbLocalsSummary() { this = "Sinatra::Base#erb" } override MethodCall getACall() { result = any(ErbCall c).asExpr().getExpr() } override predicate propagatesFlowExt(string input, string output, boolean preservesValue) { input = "Argument[locals:]" and - output = "SyntheticGlobal[" + global + "]" and + output = "SyntheticGlobal[" + any(ErbLocalsHashSyntheticGlobal global) + "]" and preservesValue = true } } + /** + * A summary for accessing a local variable in an ERB template. + * This is the second half of the modelling of the flow from the `locals` keyword argument to variables in the ERB template. + * The first half is modeled by `ErbLocalsSummary`. + */ private class ErbLocalsAccessSummary extends SummarizedCallable { private ErbLocalsHashSyntheticGlobal global; private string local; @@ -133,6 +197,8 @@ module Sinatra { } /** + * A class representing Sinatra filters AKA callbacks. + * * Filters are run before or after the route handler. They can modify the * request and response, and share instance variables with the route handler. */ @@ -166,16 +232,24 @@ module Sinatra { DataFlow::BlockNode getBody() { result = this.getBlock() } } + /** + * `before` filters run before the route handler. + */ class BeforeFilter extends Filter { BeforeFilter() { this.getMethodName() = "before" } } + /** + * `after` filters run after the route handler. + */ class AfterFilter extends Filter { AfterFilter() { this.getMethodName() = "after" } } /** * A class defining additional jump steps arising from filters. + * This only models flow between filters with no patterns - i.e. those that apply to all routes. + * Filters with patterns are not yet modeled. */ class FilterJumpStep extends DataFlowPrivate::AdditionalJumpStep { /** @@ -191,28 +265,31 @@ module Sinatra { blockCapturedSelfParameterNode(succ, route.getBody().asExpr().getExpr()) ) } - - /** - * Holds if `n` is a post-update node for the `self` parameter of `app` in block `b`. - * - * In this example, `n` is the post-update node for `@foo = 1`. - * ```rb - * class MyApp < Sinatra::Base - * before do - * @foo = 1 - * end - * end - * ``` - */ - private predicate selfPostUpdate(DataFlow::PostUpdateNode n, App app, Block b) { - n.getPreUpdateNode().asExpr().getExpr() = - any(SelfVariableAccess self | - pragma[only_bind_into](b) = self.getEnclosingCallable() and - self.getVariable().getDeclaringScope() = app.getADeclaration() - ) - } } + /** + * Holds if `n` is a post-update node for the `self` parameter of `app` in block `b`. + * + * In this example, `n` is the post-update node for `@foo = 1`. + * ```rb + * class MyApp < Sinatra::Base + * before do + * @foo = 1 + * end + * end + * ``` + */ + private predicate selfPostUpdate(DataFlow::PostUpdateNode n, App app, Block b) { + n.getPreUpdateNode().asExpr().getExpr() = + any(SelfVariableAccess self | + pragma[only_bind_into](b) = self.getEnclosingCallable() and + self.getVariable().getDeclaringScope() = app.getADeclaration() + ) + } + + /** + * Holds if `n` is a node representing the `self` parameter captured by block `b`. + */ private predicate blockCapturedSelfParameterNode(DataFlow::Node n, Block b) { exists(Ssa::CapturedSelfDefinition d | n.(DataFlowPrivate::SsaDefinitionExtNode).getDefinitionExt() = d and diff --git a/ruby/ql/test/library-tests/frameworks/sinatra/Sinatra.expected b/ruby/ql/test/library-tests/frameworks/sinatra/Sinatra.expected index 660818ec1d0..cf5df02eb18 100644 --- a/ruby/ql/test/library-tests/frameworks/sinatra/Sinatra.expected +++ b/ruby/ql/test/library-tests/frameworks/sinatra/Sinatra.expected @@ -1,23 +1,24 @@ routes -| app.rb:1:1:105:3 | MyApp | app.rb:2:3:4:5 | call to get | -| app.rb:1:1:105:3 | MyApp | app.rb:6:3:8:5 | call to get | -| app.rb:1:1:105:3 | MyApp | app.rb:10:3:13:5 | call to get | -| app.rb:1:1:105:3 | MyApp | app.rb:15:3:18:5 | call to get | -| app.rb:1:1:105:3 | MyApp | app.rb:20:3:22:5 | call to get | -| app.rb:1:1:105:3 | MyApp | app.rb:24:3:26:5 | call to get | -| app.rb:1:1:105:3 | MyApp | app.rb:28:3:31:5 | call to get | -| app.rb:1:1:105:3 | MyApp | app.rb:33:3:35:5 | call to get | -| app.rb:1:1:105:3 | MyApp | app.rb:37:3:42:5 | call to get | -| app.rb:1:1:105:3 | MyApp | app.rb:44:3:46:5 | call to get | -| app.rb:1:1:105:3 | MyApp | app.rb:48:3:50:5 | call to get | -| app.rb:1:1:105:3 | MyApp | app.rb:52:3:54:5 | call to get | -| app.rb:1:1:105:3 | MyApp | app.rb:56:3:58:5 | call to get | -| app.rb:1:1:105:3 | MyApp | app.rb:60:3:62:5 | call to get | -| app.rb:1:1:105:3 | MyApp | app.rb:66:3:68:5 | call to get | -| app.rb:1:1:105:3 | MyApp | app.rb:70:3:72:5 | call to get | -| app.rb:1:1:105:3 | MyApp | app.rb:74:3:77:5 | call to get | -| app.rb:1:1:105:3 | MyApp | app.rb:79:3:82:5 | call to get | -| app.rb:1:1:105:3 | MyApp | app.rb:89:3:92:5 | call to get | +| app.rb:1:1:114:3 | MyApp | app.rb:2:3:4:5 | call to get | +| app.rb:1:1:114:3 | MyApp | app.rb:6:3:8:5 | call to get | +| app.rb:1:1:114:3 | MyApp | app.rb:10:3:13:5 | call to get | +| app.rb:1:1:114:3 | MyApp | app.rb:15:3:18:5 | call to get | +| app.rb:1:1:114:3 | MyApp | app.rb:20:3:22:5 | call to get | +| app.rb:1:1:114:3 | MyApp | app.rb:24:3:26:5 | call to get | +| app.rb:1:1:114:3 | MyApp | app.rb:28:3:31:5 | call to get | +| app.rb:1:1:114:3 | MyApp | app.rb:33:3:35:5 | call to get | +| app.rb:1:1:114:3 | MyApp | app.rb:37:3:42:5 | call to get | +| app.rb:1:1:114:3 | MyApp | app.rb:44:3:46:5 | call to get | +| app.rb:1:1:114:3 | MyApp | app.rb:48:3:50:5 | call to get | +| app.rb:1:1:114:3 | MyApp | app.rb:52:3:54:5 | call to get | +| app.rb:1:1:114:3 | MyApp | app.rb:56:3:58:5 | call to get | +| app.rb:1:1:114:3 | MyApp | app.rb:60:3:62:5 | call to get | +| app.rb:1:1:114:3 | MyApp | app.rb:66:3:68:5 | call to get | +| app.rb:1:1:114:3 | MyApp | app.rb:70:3:72:5 | call to get | +| app.rb:1:1:114:3 | MyApp | app.rb:74:3:77:5 | call to get | +| app.rb:1:1:114:3 | MyApp | app.rb:79:3:82:5 | call to get | +| app.rb:1:1:114:3 | MyApp | app.rb:89:3:92:5 | call to get | +| app.rb:1:1:114:3 | MyApp | app.rb:94:3:96:5 | call to get | params | app.rb:3:14:3:19 | call to params | | app.rb:12:5:12:10 | call to params | @@ -34,9 +35,59 @@ erbSyntheticGlobals | SinatraErbLocalsHash(library-tests/frameworks/sinatra/views/index.erb,library-tests/frameworks/sinatra/app.rb@76:5:76:36) | views/index.erb:0:0:0:0 | views/index.erb | filters | app.rb:84:3:87:5 | call to before | before | -| app.rb:94:3:96:5 | call to after | after | -| app.rb:98:3:100:5 | call to before | before | -| app.rb:102:3:104:5 | call to after | after | +| app.rb:98:3:100:5 | call to after | after | +| app.rb:102:3:104:5 | call to before | before | +| app.rb:106:3:108:5 | call to before | before | +| app.rb:111:3:113:5 | call to after | after | filterPatterns -| app.rb:98:3:100:5 | call to before | app.rb:98:10:98:23 | "/protected/*" | -| app.rb:102:3:104:5 | call to after | app.rb:102:9:102:23 | "/create/:slug" | +| app.rb:106:3:108:5 | call to before | app.rb:106:10:106:23 | "/protected/*" | +| app.rb:111:3:113:5 | call to after | app.rb:111:9:111:23 | "/create/:slug" | +additionalFlowSteps +| app.rb:85:5:85:9 | [post] self | app.rb:2:22:4:5 | self | +| app.rb:85:5:85:9 | [post] self | app.rb:10:21:13:5 | self | +| app.rb:85:5:85:9 | [post] self | app.rb:15:23:18:5 | self | +| app.rb:85:5:85:9 | [post] self | app.rb:24:26:26:5 | self | +| app.rb:85:5:85:9 | [post] self | app.rb:37:16:42:5 | self | +| app.rb:85:5:85:9 | [post] self | app.rb:44:53:46:5 | self | +| app.rb:85:5:85:9 | [post] self | app.rb:56:32:58:5 | self | +| app.rb:85:5:85:9 | [post] self | app.rb:60:48:62:5 | self | +| app.rb:85:5:85:9 | [post] self | app.rb:74:11:77:5 | self | +| app.rb:85:5:85:9 | [post] self | app.rb:79:11:82:5 | self | +| app.rb:85:5:85:9 | [post] self | app.rb:89:16:92:5 | self | +| app.rb:85:5:85:9 | [post] self | app.rb:94:15:96:5 | self | +| app.rb:86:5:86:11 | [post] self | app.rb:2:22:4:5 | self | +| app.rb:86:5:86:11 | [post] self | app.rb:10:21:13:5 | self | +| app.rb:86:5:86:11 | [post] self | app.rb:15:23:18:5 | self | +| app.rb:86:5:86:11 | [post] self | app.rb:24:26:26:5 | self | +| app.rb:86:5:86:11 | [post] self | app.rb:37:16:42:5 | self | +| app.rb:86:5:86:11 | [post] self | app.rb:44:53:46:5 | self | +| app.rb:86:5:86:11 | [post] self | app.rb:56:32:58:5 | self | +| app.rb:86:5:86:11 | [post] self | app.rb:60:48:62:5 | self | +| app.rb:86:5:86:11 | [post] self | app.rb:74:11:77:5 | self | +| app.rb:86:5:86:11 | [post] self | app.rb:79:11:82:5 | self | +| app.rb:86:5:86:11 | [post] self | app.rb:89:16:92:5 | self | +| app.rb:86:5:86:11 | [post] self | app.rb:94:15:96:5 | self | +| app.rb:103:5:103:9 | [post] self | app.rb:2:22:4:5 | self | +| app.rb:103:5:103:9 | [post] self | app.rb:10:21:13:5 | self | +| app.rb:103:5:103:9 | [post] self | app.rb:15:23:18:5 | self | +| app.rb:103:5:103:9 | [post] self | app.rb:24:26:26:5 | self | +| app.rb:103:5:103:9 | [post] self | app.rb:37:16:42:5 | self | +| app.rb:103:5:103:9 | [post] self | app.rb:44:53:46:5 | self | +| app.rb:103:5:103:9 | [post] self | app.rb:56:32:58:5 | self | +| app.rb:103:5:103:9 | [post] self | app.rb:60:48:62:5 | self | +| app.rb:103:5:103:9 | [post] self | app.rb:74:11:77:5 | self | +| app.rb:103:5:103:9 | [post] self | app.rb:79:11:82:5 | self | +| app.rb:103:5:103:9 | [post] self | app.rb:89:16:92:5 | self | +| app.rb:103:5:103:9 | [post] self | app.rb:94:15:96:5 | self | +| app.rb:103:13:103:22 | [post] self | app.rb:2:22:4:5 | self | +| app.rb:103:13:103:22 | [post] self | app.rb:10:21:13:5 | self | +| app.rb:103:13:103:22 | [post] self | app.rb:15:23:18:5 | self | +| app.rb:103:13:103:22 | [post] self | app.rb:24:26:26:5 | self | +| app.rb:103:13:103:22 | [post] self | app.rb:37:16:42:5 | self | +| app.rb:103:13:103:22 | [post] self | app.rb:44:53:46:5 | self | +| app.rb:103:13:103:22 | [post] self | app.rb:56:32:58:5 | self | +| app.rb:103:13:103:22 | [post] self | app.rb:60:48:62:5 | self | +| app.rb:103:13:103:22 | [post] self | app.rb:74:11:77:5 | self | +| app.rb:103:13:103:22 | [post] self | app.rb:79:11:82:5 | self | +| app.rb:103:13:103:22 | [post] self | app.rb:89:16:92:5 | self | +| app.rb:103:13:103:22 | [post] self | app.rb:94:15:96:5 | self | diff --git a/ruby/ql/test/library-tests/frameworks/sinatra/Sinatra.ql b/ruby/ql/test/library-tests/frameworks/sinatra/Sinatra.ql index e3ebaf628fb..4dc3b033d6d 100644 --- a/ruby/ql/test/library-tests/frameworks/sinatra/Sinatra.ql +++ b/ruby/ql/test/library-tests/frameworks/sinatra/Sinatra.ql @@ -24,3 +24,7 @@ query predicate filters(Sinatra::Filter filter, string kind) { query predicate filterPatterns(Sinatra::Filter filter, DataFlow::ExprNode pattern) { pattern = filter.getPattern() } + +query predicate additionalFlowSteps(DataFlow::Node pred, DataFlow::Node succ) { + any(Sinatra::FilterJumpStep s).step(pred, succ) +} diff --git a/ruby/ql/test/library-tests/frameworks/sinatra/app.rb b/ruby/ql/test/library-tests/frameworks/sinatra/app.rb index 5d0698e1d00..938e11adfa4 100644 --- a/ruby/ql/test/library-tests/frameworks/sinatra/app.rb +++ b/ruby/ql/test/library-tests/frameworks/sinatra/app.rb @@ -72,7 +72,7 @@ class MyApp < Sinatra::Base end get '/' do - @foo = source "foo" + @foo = params["foo"] erb :index, locals: {foo: @foo} end