Merge pull request #7061 from github/hmac/actiondispatch

Ruby: Rails route resolution
This commit is contained in:
Harry Maclean
2022-02-10 09:46:36 +13:00
committed by GitHub
14 changed files with 1182 additions and 15 deletions

View File

@@ -13,3 +13,4 @@ private import codeql.ruby.frameworks.StandardLibrary
private import codeql.ruby.frameworks.Files
private import codeql.ruby.frameworks.HttpClients
private import codeql.ruby.frameworks.XmlParsing
private import codeql.ruby.frameworks.ActionDispatch

View File

@@ -6,6 +6,7 @@ private import codeql.ruby.dataflow.RemoteFlowSources
private import codeql.ruby.ast.internal.Module
private import codeql.ruby.ApiGraphs
private import ActionView
private import codeql.ruby.frameworks.ActionDispatch
/**
* A `ClassDeclaration` for a class that extends `ActionController::Base`.
@@ -69,6 +70,26 @@ class ActionControllerActionMethod extends Method, HTTP::Server::RequestHandler:
// not end at an explicit render or redirect
/** Gets the controller class containing this method. */
ActionControllerControllerClass getControllerClass() { result = controllerClass }
/**
* Gets a route to this handler, if one exists.
* May return multiple results.
*/
ActionDispatch::Route getARoute() {
exists(string name |
isRoute(result, name, controllerClass) and
isActionControllerMethod(this, name, controllerClass)
)
}
}
pragma[nomagic]
private predicate isRoute(
ActionDispatch::Route route, string name, ActionControllerControllerClass controllerClass
) {
route.getController() + "_controller" =
ActionDispatch::underscore(namespaceDeclaration(controllerClass)) and
name = route.getAction()
}
// A method call with a `self` receiver from within a controller class

View File

@@ -0,0 +1,951 @@
/**
* Models routing configuration specified using the `ActionDispatch` library, which is part of Rails.
*/
private import codeql.ruby.AST
private import codeql.ruby.Concepts
private import codeql.ruby.DataFlow
/**
* Models routing configuration specified using the `ActionDispatch` library, which is part of Rails.
*/
module ActionDispatch {
/**
* A block that defines some routes.
* Route blocks can contribute to the path or controller namespace of their child routes.
* For example, in the block below
* ```rb
* scope path: "/admin" do
* get "/dashboard", to: "admin_dashboard#show"
* end
* ```
* the route defined by the call to `get` has the full path `/admin/dashboard`.
* We track these contributions via `getPathComponent` and `getControllerComponent`.
*/
abstract private class RouteBlock extends TRouteBlock {
/**
* Gets the name of a primary CodeQL class to which this route block belongs.
*/
string getAPrimaryQlClass() { result = "RouteBlock" }
/**
* Gets a string representation of this route block.
*/
string toString() { none() }
/**
* Gets a `Stmt` within this route block.
*/
abstract Stmt getAStmt();
/**
* Gets the parent of this route block, if one exists.
*/
abstract RouteBlock getParent();
/**
* Gets the `n`th parent of this route block.
* The zeroth parent is this block, the first parent is the direct parent of this block, etc.
*/
RouteBlock getParent(int n) {
if n = 0 then result = this else result = this.getParent().getParent(n - 1)
}
/**
* Gets the component of the path defined by this block, if it exists.
*/
abstract string getPathComponent();
/**
* Gets the component of the controller namespace defined by this block, if it exists.
*/
abstract string getControllerComponent();
/**
* Gets the location of this route block.
*/
abstract Location getLocation();
}
/**
* A route block that is not the top-level block.
* This block will always have a parent.
*/
abstract private class NestedRouteBlock extends RouteBlock {
RouteBlock parent;
override RouteBlock getParent() { result = parent }
override string getAPrimaryQlClass() { result = "NestedRouteBlock" }
}
/**
* A top-level routes block.
* ```rb
* Rails.application.routes.draw do
* ...
* end
* ```
*/
private class TopLevelRouteBlock extends RouteBlock, TTopLevelRouteBlock {
MethodCall call;
// Routing blocks create scopes which define the namespace for controllers and paths,
// though they can be overridden in various ways.
// The namespaces can differ, so we track them separately.
Block block;
TopLevelRouteBlock() { this = TTopLevelRouteBlock(_, call, block) }
override string getAPrimaryQlClass() { result = "TopLevelRouteBlock" }
Block getBlock() { result = block }
override Stmt getAStmt() { result = block.getAStmt() }
override RouteBlock getParent() { none() }
override string toString() { result = call.toString() }
override Location getLocation() { result = call.getLocation() }
override string getPathComponent() { none() }
override string getControllerComponent() { none() }
}
/**
* A route block defined by a call to `constraints`.
* ```rb
* constraints(foo: /some_regex/) do
* get "/posts/:foo", to "posts#something"
* end
* ```
* https://api.rubyonrails.org/classes/ActionDispatch/Routing/Mapper/Scoping.html#method-i-constraints
*/
private class ConstraintsRouteBlock extends NestedRouteBlock, TConstraintsRouteBlock {
private Block block;
private MethodCall call;
ConstraintsRouteBlock() { this = TConstraintsRouteBlock(parent, call, block) }
override string getAPrimaryQlClass() { result = "ConstraintsRouteBlock" }
override Stmt getAStmt() { result = block.getAStmt() }
override string getPathComponent() { result = "" }
override string getControllerComponent() { result = "" }
override string toString() { result = call.toString() }
override Location getLocation() { result = call.getLocation() }
}
/**
* A route block defined by a call to `scope`.
* ```rb
* scope(path: "/some_path", module: "some_module") do
* get "/posts/:foo", to "posts#something"
* end
* ```
* https://api.rubyonrails.org/classes/ActionDispatch/Routing/Mapper/Scoping.html#method-i-scope
*/
private class ScopeRouteBlock extends NestedRouteBlock, TScopeRouteBlock {
private MethodCall call;
private Block block;
ScopeRouteBlock() { this = TScopeRouteBlock(parent, call, block) }
override string getAPrimaryQlClass() { result = "ScopeRouteBlock" }
override Stmt getAStmt() { result = block.getAStmt() }
override string toString() { result = call.toString() }
override Location getLocation() { result = call.getLocation() }
override string getPathComponent() {
call.getKeywordArgument("path").getConstantValue().isStringOrSymbol(result)
or
not exists(call.getKeywordArgument("path")) and
call.getArgument(0).getConstantValue().isStringOrSymbol(result)
}
override string getControllerComponent() {
call.getKeywordArgument(["controller", "module"]).getConstantValue().isStringOrSymbol(result)
}
}
/**
* A route block defined by a call to `resources`.
* ```rb
* resources :articles do
* get "/comments", to "comments#index"
* end
* ```
* https://api.rubyonrails.org/classes/ActionDispatch/Routing/Mapper/Resources.html#method-i-resources
*/
private class ResourcesRouteBlock extends NestedRouteBlock, TResourcesRouteBlock {
private MethodCall call;
private Block block;
ResourcesRouteBlock() { this = TResourcesRouteBlock(parent, call, block) }
override string getAPrimaryQlClass() { result = "ResourcesRouteBlock" }
override Stmt getAStmt() { result = block.getAStmt() }
/**
* Gets the `resources` call that gives rise to this route block.
*/
MethodCall getDefiningMethodCall() { result = call }
override string getPathComponent() {
exists(string resource | call.getArgument(0).getConstantValue().isStringOrSymbol(resource) |
result = resource + "/:" + singularize(resource) + "_id"
)
}
override string getControllerComponent() { result = "" }
override string toString() { result = call.toString() }
override Location getLocation() { result = call.getLocation() }
}
/**
* A route block that is guarded by a conditional statement.
* For example:
* ```rb
* if Rails.env.test?
* get "/foo/bar", to: "foo#bar"
* end
* ```
* We ignore the condition and analyze both branches to obtain as
* much routing information as possible.
*/
private class ConditionalRouteBlock extends NestedRouteBlock, TConditionalRouteBlock {
private ConditionalExpr e;
ConditionalRouteBlock() { this = TConditionalRouteBlock(parent, e) }
override string getAPrimaryQlClass() { result = "ConditionalRouteBlock" }
override Stmt getAStmt() { result = e.getBranch(_).(StmtSequence).getAStmt() }
override string getPathComponent() { none() }
override string getControllerComponent() { none() }
override string toString() { result = e.toString() }
override Location getLocation() { result = e.getLocation() }
}
/**
* A route block defined by a call to `namespace`.
* ```rb
* namespace :admin do
* resources :posts
* end
* ```
* https://api.rubyonrails.org/classes/ActionDispatch/Routing/Mapper/Scoping.html#method-i-namespace
*/
private class NamespaceRouteBlock extends NestedRouteBlock, TNamespaceRouteBlock {
private MethodCall call;
private Block block;
NamespaceRouteBlock() { this = TNamespaceRouteBlock(parent, call, block) }
override Stmt getAStmt() { result = block.getAStmt() }
override string getPathComponent() { result = this.getNamespace() }
override string getControllerComponent() { result = this.getNamespace() }
private string getNamespace() {
call.getArgument(0).getConstantValue().isStringOrSymbol(result)
}
override string toString() { result = call.toString() }
override Location getLocation() { result = call.getLocation() }
}
/**
* A route configuration. This defines a combination of HTTP method and URL
* path which should be routed to a particular controller-action pair.
* This can arise from an explicit call to a routing method, for example:
* ```rb
* get "/photos", to: "photos#index"
* ```
* or via a convenience method like `resources`, which defines mutiple routes at once:
* ```rb
* resources :photos
* ```
*/
class Route extends TRoute instanceof RouteImpl {
/**
* Gets the name of a primary CodeQL class to which this route belongs.
*/
string getAPrimaryQlClass() { result = "Route" }
/** Gets a string representation of this route. */
string toString() { result = super.toString() }
/**
* Gets the location of the method call that defines this route.
*/
Location getLocation() { result = super.getLocation() }
/**
* Gets the full controller targeted by this route.
*/
string getController() { result = super.getController() }
/**
* Gets the action targeted by this route.
*/
string getAction() { result = super.getAction() }
/**
* Gets the HTTP method of this route.
* The result is one of [get, post, put, patch, delete].
*/
string getHttpMethod() { result = super.getHttpMethod() }
/**
* Gets the full path of the route.
*/
string getPath() { result = super.getPath() }
/**
* Get a URL capture. This is a wildcard URL segment whose value is placed in `params`.
* For example, in
* ```ruby
* get "/foo/:bar/baz", to: "users#index"
* ```
* the capture is `:bar`.
*/
string getACapture() { result = super.getACapture() }
}
/**
* The implementation of `Route`.
* This class is abstract and is thus kept private so we don't expose it to
* users.
* Extend this class to add new instances of routes.
*/
abstract private class RouteImpl extends TRoute {
/**
* Gets the name of a primary CodeQL class to which this route belongs.
*/
string getAPrimaryQlClass() { result = "RouteImpl" }
MethodCall method;
/** Gets a string representation of this route. */
string toString() { result = method.toString() }
/**
* Gets the location of the method call that defines this route.
*/
Location getLocation() { result = method.getLocation() }
/**
* Gets the method call that defines this route.
*/
MethodCall getDefiningMethodCall() { result = method }
/**
* Get the last component of the path. For example, in
* ```rb
* get "/photos", to: "photos#index"
* ```
* this is `/photos`.
* If the string has any interpolations, this predicate will have no result.
*/
abstract string getLastPathComponent();
/**
* Gets the HTTP method of this route.
* The result is one of [get, post, put, patch, delete].
*/
abstract string getHttpMethod();
/**
* Gets the last controller component.
* This is the controller specified in the route itself.
*/
abstract string getLastControllerComponent();
/**
* Gets a component of the controller.
* This behaves identically to `getPathComponent`, but for controller information.
*/
string getControllerComponent(int n) {
if n = 0
then result = this.getLastControllerComponent()
else result = this.getParentBlock().getParent(n - 1).getControllerComponent()
}
/**
* Gets the full controller targeted by this route.
*/
string getController() {
result =
concat(int n |
this.getControllerComponent(n) != ""
|
this.getControllerComponent(n), "/" order by n desc
)
}
/**
* Gets the action targeted by this route.
*/
abstract string getAction();
/**
* Gets the parent `RouteBlock` of this route.
*/
abstract RouteBlock getParentBlock();
/**
* Gets a component of the path. Components are numbered from 0 up, where 0
* is the last component, 1 is the second-last, etc.
* For example, in the following route:
*
* ```rb
* namespace path: "foo" do
* namespace path: "bar" do
* get "baz", to: "foo#bar
* end
* end
* ```
*
* the components are:
*
* | n | component
* |---|----------
* | 0 | baz
* | 1 | bar
* | 2 | foo
*/
string getPathComponent(int n) {
if n = 0
then result = this.getLastPathComponent()
else result = this.getParentBlock().getParent(n - 1).getPathComponent()
}
/**
* Gets the full path of the route.
*/
string getPath() {
result =
concat(int n |
this.getPathComponent(n) != ""
|
// Strip leading and trailing slashes from each path component before combining
stripSlashes(this.getPathComponent(n)), "/" order by n desc
)
}
/**
* Get a URL capture. This is a wildcard URL segment whose value is placed in `params`.
* For example, in
* ```ruby
* get "/foo/:bar/baz", to: "users#index"
* ```
* the capture is `:bar`.
* We don't currently make use of this, but it may be useful in future to more accurately
* model the contents of the `params` hash.
*/
string getACapture() { result = this.getPathComponent(_).regexpFind(":[^:/]+", _, _) }
}
/**
* A route generated by an explicit call to `get`, `post`, etc.
*
* ```ruby
* get "/photos", to: "photos#index"
* put "/photos/:id", to: "photos#update"
* ```
*/
private class ExplicitRoute extends RouteImpl, TExplicitRoute {
RouteBlock parentBlock;
ExplicitRoute() { this = TExplicitRoute(parentBlock, method) }
override string getAPrimaryQlClass() { result = "ExplicitRoute" }
override RouteBlock getParentBlock() { result = parentBlock }
override string getLastPathComponent() {
method.getArgument(0).getConstantValue().isStringOrSymbol(result)
}
override string getLastControllerComponent() {
method.getKeywordArgument("controller").getConstantValue().isStringOrSymbol(result)
or
not exists(method.getKeywordArgument("controller")) and
(
result = extractController(this.getActionString())
or
// If controller is not specified, and we're in a `resources` route block, use the controller of that route.
// For example, in
//
// resources :posts do
// get "timestamp", to: :timestamp
// end
//
// The route is GET /posts/:post_id/timestamp => posts/timestamp
not exists(extractController(this.getActionString())) and
exists(ResourcesRoute r |
r.getDefiningMethodCall() = parentBlock.(ResourcesRouteBlock).getDefiningMethodCall()
|
result = r.getLastControllerComponent()
)
)
}
private string getActionString() {
method.getKeywordArgument("to").getConstantValue().isStringOrSymbol(result)
or
method.getKeywordArgument("to").(MethodCall).getMethodName() = "redirect" and
result = "<redirect>#<redirect>"
}
override string getAction() {
// get "/photos", action: "index"
method.getKeywordArgument("action").getConstantValue().isStringOrSymbol(result)
or
not exists(method.getKeywordArgument("action")) and
(
// get "/photos", to: "photos#index"
// get "/photos", to: redirect("some_url")
result = extractAction(this.getActionString())
or
// resources :photos, only: [] do
// get "/", to: "index"
// end
not exists(extractAction(this.getActionString())) and result = this.getActionString()
or
// get :some_action
not exists(this.getActionString()) and
method.getArgument(0).getConstantValue().isStringOrSymbol(result)
)
}
override string getHttpMethod() { result = method.getMethodName().toString() }
}
/**
* A route generated by a call to `resources`.
*
* ```ruby
* resources :photos
* ```
* This creates eight routes, equivalent to the following code:
* ```ruby
* get "/photos", to: "photos#index"
* get "/photos/new", to: "photos#new"
* post "/photos", to: "photos#create"
* get "/photos/:id", to: "photos#show"
* get "/photos/:id/edit", to: "photos#edit"
* patch "/photos/:id", to: "photos#update"
* put "/photos/:id", to: "photos#update"
* delete "/photos/:id", to: "photos#delete"
* ```
*
* `resources` can take a block. Any routes defined inside the block will inherit a path component of
* `/<resource>/:<resource>_id`. For example:
*
* ```ruby
* resources :photos do
* get "/foo", to: "photos#foo"
* end
* ```
* This creates the eight default routes, plus one more, which is nested under "/photos/:photo_id", equivalent to:
* ```ruby
* get "/photos/:photo_id/foo", to: "photos#foo"
* ```
*/
private class ResourcesRoute extends RouteImpl, TResourcesRoute {
RouteBlock parent;
string resource;
string action;
string httpMethod;
string pathComponent;
ResourcesRoute() {
this = TResourcesRoute(parent, method, action) and
method.getArgument(0).getConstantValue().isStringOrSymbol(resource) and
isDefaultResourceRoute(resource, httpMethod, pathComponent, action)
}
override string getAPrimaryQlClass() { result = "ResourcesRoute" }
override RouteBlock getParentBlock() { result = parent }
override string getLastPathComponent() { result = pathComponent }
override string getLastControllerComponent() {
method.getArgument(0).getConstantValue().isStringOrSymbol(result)
}
override string getAction() { result = action }
override string getHttpMethod() { result = httpMethod }
}
/**
* A route generated by a call to `resource`.
* This is like a `resources` route, but creates routes for a singular resource.
* This means there's no index route, no id parameter, and the resource name is expected to be singular.
* It will still be routed to a pluralised controller name.
* ```ruby
* resource :account
* ```
*/
private class SingularResourceRoute extends RouteImpl, TResourceRoute {
RouteBlock parent;
string resource;
string action;
string httpMethod;
string pathComponent;
SingularResourceRoute() {
this = TResourceRoute(parent, method, action) and
method.getArgument(0).getConstantValue().isStringOrSymbol(resource) and
isDefaultSingularResourceRoute(resource, httpMethod, pathComponent, action)
}
override string getAPrimaryQlClass() { result = "SingularResourceRoute" }
override RouteBlock getParentBlock() { result = parent }
override string getLastPathComponent() { result = pathComponent }
override string getLastControllerComponent() {
method.getArgument(0).getConstantValue().isStringOrSymbol(result)
}
override string getAction() { result = action }
override string getHttpMethod() { result = httpMethod }
}
/**
* A route generated by a call to `match`.
* This is a lower level primitive that powers `get`, `post` etc.
* The first argument can be a path or a (path, controller-action) pair.
* The controller, action and HTTP method can be specified with the
* `controller:`, `action:` and `via:` keyword arguments, respectively.
* ```ruby
* match 'photos/:id' => 'photos#show', via: :get
* match 'photos/:id', to: 'photos#show', via: :get
* match 'photos/:id', to 'photos#show', via: [:get, :post]
* match 'photos/:id', controller: 'photos', action: 'show', via: :get
* ```
*/
private class MatchRoute extends RouteImpl, TMatchRoute {
private RouteBlock parent;
MatchRoute() { this = TMatchRoute(parent, method) }
override string getAPrimaryQlClass() { result = "MatchRoute" }
override RouteBlock getParentBlock() { result = parent }
override string getLastPathComponent() {
[method.getArgument(0), method.getArgument(0).(Pair).getKey()]
.getConstantValue()
.isStringOrSymbol(result)
}
override string getLastControllerComponent() {
result =
extractController(method.getKeywordArgument("to").getConstantValue().getStringOrSymbol()) or
method.getKeywordArgument("controller").getConstantValue().isStringOrSymbol(result) or
result =
extractController(method
.getArgument(0)
.(Pair)
.getValue()
.getConstantValue()
.getStringOrSymbol())
}
override string getHttpMethod() {
exists(string via |
method.getKeywordArgument("via").getConstantValue().isStringOrSymbol(via)
|
via = "all" and result = anyHttpMethod()
or
via != "all" and result = via
)
or
result =
method
.getKeywordArgument("via")
.(ArrayLiteral)
.getElement(_)
.getConstantValue()
.getStringOrSymbol()
}
override string getAction() {
result = extractAction(method.getKeywordArgument("to").getConstantValue().getStringOrSymbol()) or
method.getKeywordArgument("action").getConstantValue().isStringOrSymbol(result) or
result =
extractAction(method.getArgument(0).(Pair).getValue().getConstantValue().getStringOrSymbol())
}
}
private import Cached
/**
* This module contains the IPA types backing `RouteBlock` and `Route`, cached for performance.
*/
cached
private module Cached {
cached
newtype TRouteBlock =
TTopLevelRouteBlock(MethodCall routes, MethodCall draw, Block b) {
routes.getMethodName() = "routes" and
draw.getMethodName() = "draw" and
draw.getReceiver() = routes and
draw.getBlock() = b
} or
// constraints(foo: /some_regex/) do
// get "/posts/:foo", to "posts#something"
// end
TConstraintsRouteBlock(RouteBlock parent, MethodCall constraints, Block b) {
parent.getAStmt() = constraints and
constraints.getMethodName() = "constraints" and
constraints.getBlock() = b
} or
// scope(path: "/some_path", module: "some_module") do
// get "/posts/:foo", to "posts#something"
// end
TScopeRouteBlock(RouteBlock parent, MethodCall scope, Block b) {
parent.getAStmt() = scope and scope.getMethodName() = "scope" and scope.getBlock() = b
} or
// resources :articles do
// get "/comments", to "comments#index"
// end
TResourcesRouteBlock(RouteBlock parent, MethodCall resources, Block b) {
parent.getAStmt() = resources and
resources.getMethodName() = "resources" and
resources.getBlock() = b
} or
// A conditional statement guarding some routes.
// We ignore the condition and analyze both branches to obtain as
// much routing information as possible.
TConditionalRouteBlock(RouteBlock parent, ConditionalExpr e) { parent.getAStmt() = e } or
// namespace :admin do
// resources :posts
// end
TNamespaceRouteBlock(RouteBlock parent, MethodCall namespace, Block b) {
parent.getAStmt() = namespace and
namespace.getMethodName() = "namespace" and
namespace.getBlock() = b
}
/**
* A route configuration. See `Route` for more info
*/
cached
newtype TRoute =
/**
* See `ExplicitRoute`
*/
TExplicitRoute(RouteBlock b, MethodCall m) {
b.getAStmt() = m and m.getMethodName() = anyHttpMethod()
} or
/**
* See `ResourcesRoute`
*/
TResourcesRoute(RouteBlock b, MethodCall m, string action) {
b.getAStmt() = m and
m.getMethodName() = "resources" and
action in ["show", "index", "new", "edit", "create", "update", "destroy"] and
applyActionFilters(m, action)
} or
/**
* See `SingularResourceRoute`
*/
TResourceRoute(RouteBlock b, MethodCall m, string action) {
b.getAStmt() = m and
m.getMethodName() = "resource" and
action in ["show", "new", "edit", "create", "update", "destroy"] and
applyActionFilters(m, action)
} or
/**
* See `MatchRoute`
*/
TMatchRoute(RouteBlock b, MethodCall m) { b.getAStmt() = m and m.getMethodName() = "match" }
}
/**
* Several routing methods support the keyword arguments `only:` and `except:`.
* - `only:` restricts the set of actions to just those in the argument.
* - `except:` removes the given actions from the set.
*/
bindingset[action]
private predicate applyActionFilters(MethodCall m, string action) {
// Respect the `only` keyword argument, which restricts the set of actions.
(
not exists(m.getKeywordArgument("only"))
or
exists(Expr only | only = m.getKeywordArgument("only") |
[only.(ArrayLiteral).getElement(_), only].getConstantValue().isStringOrSymbol(action)
)
) and
// Respect the `except` keyword argument, which removes actions from the default set.
(
not exists(m.getKeywordArgument("except"))
or
exists(Expr except | except = m.getKeywordArgument("except") |
[except.(ArrayLiteral).getElement(_), except].getConstantValue().getStringOrSymbol() !=
action
)
)
}
/**
* Holds if the (resource, method, path, action) combination would be generated by a call to `resources :<resource>`.
*/
bindingset[resource]
private predicate isDefaultResourceRoute(
string resource, string method, string path, string action
) {
action = "create" and
(method = "post" and path = "/" + resource)
or
action = "index" and
(method = "get" and path = "/" + resource)
or
action = "new" and
(method = "get" and path = "/" + resource + "/new")
or
action = "edit" and
(method = "get" and path = "/" + resource + ":id/edit")
or
action = "show" and
(method = "get" and path = "/" + resource + "/:id")
or
action = "update" and
(method in ["put", "patch"] and path = "/" + resource + "/:id")
or
action = "destroy" and
(method = "delete" and path = "/" + resource + "/:id")
}
/**
* Holds if the (resource, method, path, action) combination would be generated by a call to `resource :<resource>`.
*/
bindingset[resource]
private predicate isDefaultSingularResourceRoute(
string resource, string method, string path, string action
) {
action = "create" and
(method = "post" and path = "/" + resource)
or
action = "new" and
(method = "get" and path = "/" + resource + "/new")
or
action = "edit" and
(method = "get" and path = "/" + resource + "/edit")
or
action = "show" and
(method = "get" and path = "/" + resource)
or
action = "update" and
(method in ["put", "patch"] and path = "/" + resource)
or
action = "destroy" and
(method = "delete" and path = "/" + resource)
}
/**
* Extract the controller from a Rails routing string
* ```
* extractController("posts#show") = "posts"
* ```
*/
bindingset[input]
private string extractController(string input) { result = input.regexpCapture("([^#]+)#.+", 1) }
/**
* Extract the action from a Rails routing string
* ```
* extractAction("posts#show") = "show"
*/
bindingset[input]
private string extractAction(string input) { result = input.regexpCapture("[^#]+#(.+)", 1) }
/**
* Returns the lowercase name of every HTTP method we support.
*/
private string anyHttpMethod() { result = ["get", "post", "put", "patch", "delete"] }
/**
* The inverse of `pluralize`
* photos => photo
* stories => story
* not_plural => not_plural
*/
bindingset[input]
private string singularize(string input) {
exists(string prefix | prefix = input.regexpCapture("(.*)ies", 1) | result = prefix + "y")
or
not input.matches("%ies") and
exists(string prefix | prefix = input.regexpCapture("(.*)s", 1) | result = prefix)
or
not input.regexpMatch(".*(ies|s)") and result = input
}
/**
* Convert a camel-case string to underscore case. Converts `::` to `/`.
* This can be used to convert ActiveRecord controller names to a canonical form that matches the routes they handle.
* Note: All-uppercase words like `CONSTANT` are not handled correctly.
*/
bindingset[base]
string underscore(string base) {
base = "" and result = ""
or
result =
base.charAt(0).toLowerCase() +
// Walk along the string, keeping track of the previous character
// in order to determine if we've crossed a boundary.
// Boundaries are:
// - lower case to upper case: B in FooBar
// - entering a namespace: B in Foo::Bar
concat(int i, string prev, string char |
prev = base.charAt(i) and
char = base.charAt(i + 1)
|
any(string s |
char.regexpMatch("[A-Za-z0-9]") and
if prev = ":"
then s = "/" + char.toLowerCase()
else
if prev.isLowercase() and char.isUppercase()
then s = "_" + char.toLowerCase()
else s = char.toLowerCase()
)
order by
i
)
}
/**
* Strip leading and trailing forward slashes from the string.
*/
bindingset[input]
private string stripSlashes(string input) {
result = input.regexpReplaceAll("^/+(.+)$", "$1").regexpReplaceAll("^(.*[^/])/+$", "$1")
}
}

View File

@@ -10,6 +10,7 @@ private import codeql.ruby.Concepts
private import codeql.ruby.dataflow.RemoteFlowSources
private import codeql.ruby.dataflow.BarrierGuards
private import codeql.ruby.dataflow.Sanitizers
private import codeql.ruby.frameworks.ActionController
/**
* Provides default sources, sinks and sanitizers for detecting
@@ -54,15 +55,21 @@ module UrlRedirect {
*/
class RedirectLocationAsSink extends Sink {
RedirectLocationAsSink() {
exists(HTTP::Server::HttpRedirectResponse e |
exists(HTTP::Server::HttpRedirectResponse e, MethodBase method |
this = e.getRedirectLocation() and
// As a rough heuristic, assume that methods with these names are handlers for POST/PUT/PATCH/DELETE requests,
// which are not as vulnerable to URL redirection because browsers will not initiate them from clicking a link.
not this.asExpr()
.getExpr()
.getEnclosingMethod()
.getName()
.regexpMatch(".*(create|update|destroy).*")
// We only want handlers for GET requests.
// Handlers for other HTTP methods are not as vulnerable to URL
// redirection as browsers will not initiate them from clicking a link.
method = this.asExpr().getExpr().getEnclosingMethod() and
(
// If there's a Rails GET route to this handler, we can be certain that it is a candidate.
method.(ActionControllerActionMethod).getARoute().getHttpMethod() = "get"
or
// Otherwise, we have to rely on a heuristic to filter out invulnerable handlers.
// We exclude any handlers with names containing create/update/destroy, as these are not likely to handle GET requests.
not exists(method.(ActionControllerActionMethod).getARoute()) and
not method.getName().regexpMatch(".*(create|update|destroy).*")
)
)
}
}

View File

@@ -2,15 +2,26 @@ actionControllerControllerClasses
| ActiveRecordInjection.rb:27:1:58:3 | FooController |
| ActiveRecordInjection.rb:60:1:90:3 | BarController |
| ActiveRecordInjection.rb:92:1:96:3 | BazController |
| app/controllers/comments_controller.rb:1:1:7:3 | CommentsController |
| app/controllers/foo/bars_controller.rb:3:1:31:3 | BarsController |
| app/controllers/photos_controller.rb:1:1:4:3 | PhotosController |
| app/controllers/posts_controller.rb:1:1:10:3 | PostsController |
| app/controllers/users/notifications_controller.rb:2:3:5:5 | NotificationsController |
actionControllerActionMethods
| ActiveRecordInjection.rb:32:3:57:5 | some_request_handler |
| ActiveRecordInjection.rb:61:3:69:5 | some_other_request_handler |
| ActiveRecordInjection.rb:71:3:89:5 | safe_paths |
| ActiveRecordInjection.rb:93:3:95:5 | yet_another_handler |
| app/controllers/comments_controller.rb:2:3:3:5 | index |
| app/controllers/comments_controller.rb:5:3:6:5 | show |
| app/controllers/foo/bars_controller.rb:5:3:7:5 | index |
| app/controllers/foo/bars_controller.rb:9:3:18:5 | show_debug |
| app/controllers/foo/bars_controller.rb:20:3:24:5 | show |
| app/controllers/photos_controller.rb:2:3:3:5 | show |
| app/controllers/posts_controller.rb:2:3:3:5 | index |
| app/controllers/posts_controller.rb:5:3:6:5 | show |
| app/controllers/posts_controller.rb:8:3:9:5 | upvote |
| app/controllers/users/notifications_controller.rb:3:5:4:7 | mark_as_read |
paramsCalls
| ActiveRecordInjection.rb:35:30:35:35 | call to params |
| ActiveRecordInjection.rb:39:29:39:34 | call to params |

View File

@@ -0,0 +1,56 @@
actionDispatchRoutes
| app/config/routes.rb:2:3:8:5 | call to resources | get | posts | posts | index |
| app/config/routes.rb:2:3:8:5 | call to resources | get | posts/:id | posts | show |
| app/config/routes.rb:3:5:6:7 | call to resources | delete | posts/:post_id/comments/:id | comments | destroy |
| app/config/routes.rb:3:5:6:7 | call to resources | get | posts/:post_id/comments | comments | index |
| app/config/routes.rb:3:5:6:7 | call to resources | get | posts/:post_id/comments/:id | comments | show |
| app/config/routes.rb:3:5:6:7 | call to resources | get | posts/:post_id/comments/new | comments | new |
| app/config/routes.rb:3:5:6:7 | call to resources | get | posts/:post_id/comments:id/edit | comments | edit |
| app/config/routes.rb:3:5:6:7 | call to resources | patch | posts/:post_id/comments/:id | comments | update |
| app/config/routes.rb:3:5:6:7 | call to resources | post | posts/:post_id/comments | comments | create |
| app/config/routes.rb:3:5:6:7 | call to resources | put | posts/:post_id/comments/:id | comments | update |
| app/config/routes.rb:4:7:4:41 | call to resources | post | posts/:post_id/comments/:comment_id/replies | replies | create |
| app/config/routes.rb:5:7:5:28 | call to post | post | posts/:post_id/comments/:comment_id/flag | comments | flag |
| app/config/routes.rb:7:5:7:37 | call to post | post | posts/:post_id/upvote | posts | upvote |
| app/config/routes.rb:11:5:11:54 | call to post | post | destroy_all_posts | posts | destroy_alll |
| app/config/routes.rb:15:5:15:46 | call to get | get | numbers/:number | numbers | show |
| app/config/routes.rb:19:5:19:44 | call to get | get | admin/jobs | background_jobs | index |
| app/config/routes.rb:23:5:23:64 | call to get | get | admin/secrets | secrets | view_secrets |
| app/config/routes.rb:24:5:24:42 | call to delete | delete | admin/:user_id | users | destroy |
| app/config/routes.rb:27:3:27:48 | call to match | get | photos/:id | photos | show |
| app/config/routes.rb:28:3:28:50 | call to match | get | photos/:id | photos | show |
| app/config/routes.rb:29:3:29:69 | call to match | get | photos/:id | photos | show |
| app/config/routes.rb:30:3:30:50 | call to match | delete | photos/:id | photos | show |
| app/config/routes.rb:30:3:30:50 | call to match | get | photos/:id | photos | show |
| app/config/routes.rb:30:3:30:50 | call to match | patch | photos/:id | photos | show |
| app/config/routes.rb:30:3:30:50 | call to match | post | photos/:id | photos | show |
| app/config/routes.rb:30:3:30:50 | call to match | put | photos/:id | photos | show |
| app/config/routes.rb:33:5:33:43 | call to post | post | upgrade | users | start_upgrade |
| app/config/routes.rb:37:5:37:31 | call to get | get | current_billing_cycle | billing/enterprise | current_billing_cycle |
| app/config/routes.rb:40:3:40:40 | call to resource | get | global_config | global_config | show |
| app/config/routes.rb:43:5:45:7 | call to resources | get | foo/bar | foo/bar | index |
| app/config/routes.rb:43:5:45:7 | call to resources | get | foo/bar/:id | foo/bar | show |
| app/config/routes.rb:44:7:44:39 | call to get | get | foo/bar/:bar_id/show_debug | foo/bar | show_debug |
| app/config/routes.rb:49:5:49:95 | call to delete | delete | users/:user/notifications | users/notifications | destroy |
| app/config/routes.rb:50:5:50:94 | call to post | post | users/:user/notifications/:notification_id/mark_as_read | users/notifications | mark_as_read |
actionDispatchControllerMethods
| app/config/routes.rb:2:3:8:5 | call to resources | app/controllers/posts_controller.rb:2:3:3:5 | index |
| app/config/routes.rb:2:3:8:5 | call to resources | app/controllers/posts_controller.rb:5:3:6:5 | show |
| app/config/routes.rb:3:5:6:7 | call to resources | app/controllers/comments_controller.rb:2:3:3:5 | index |
| app/config/routes.rb:3:5:6:7 | call to resources | app/controllers/comments_controller.rb:5:3:6:5 | show |
| app/config/routes.rb:7:5:7:37 | call to post | app/controllers/posts_controller.rb:8:3:9:5 | upvote |
| app/config/routes.rb:27:3:27:48 | call to match | app/controllers/photos_controller.rb:2:3:3:5 | show |
| app/config/routes.rb:28:3:28:50 | call to match | app/controllers/photos_controller.rb:2:3:3:5 | show |
| app/config/routes.rb:29:3:29:69 | call to match | app/controllers/photos_controller.rb:2:3:3:5 | show |
| app/config/routes.rb:30:3:30:50 | call to match | app/controllers/photos_controller.rb:2:3:3:5 | show |
| app/config/routes.rb:50:5:50:94 | call to post | app/controllers/users/notifications_controller.rb:3:5:4:7 | mark_as_read |
underscore
| Foo | foo |
| Foo::Bar | foo/bar |
| Foo::Bar::Baz | foo/bar/baz |
| Foo::Bar::BazQuux | foo/bar/baz_quux |
| FooBar | foo_bar |
| FooBar::Baz | foo_bar/baz |
| HTTPServerRequest | httpserver_request |
| LotsOfCapitalLetters | lots_of_capital_letters |
| invalid | invalid |

View File

@@ -0,0 +1,26 @@
private import ruby
private import codeql.ruby.frameworks.ActionDispatch
private import codeql.ruby.frameworks.ActionController
query predicate actionDispatchRoutes(
ActionDispatch::Route r, string method, string path, string controller, string action
) {
r.getHttpMethod() = method and
r.getPath() = path and
r.getController() = controller and
r.getAction() = action
}
query predicate actionDispatchControllerMethods(
ActionDispatch::Route r, ActionControllerActionMethod m
) {
m.getARoute() = r
}
query predicate underscore(string input, string output) {
output = ActionDispatch::underscore(input) and
input in [
"Foo", "FooBar", "Foo::Bar", "FooBar::Baz", "Foo::Bar::Baz", "Foo::Bar::BazQuux", "invalid",
"HTTPServerRequest", "LotsOfCapitalLetters"
]
}

View File

@@ -0,0 +1,52 @@
Rails.application.routes.draw do
resources :posts, only: [:show, :index] do
resources :comments do
resources :replies, only: [:create]
post "flag", to: :flag
end
post "upvote", to: "posts#upvote"
end
if Rails.env.test?
post "destroy_all_posts", to: "posts#destroy_alll"
end
constraints(number: /[0-9]+/) do
get "/numbers/:number", to: "numbers#show"
end
scope path: "/admin" do
get "/jobs", to: "background_jobs#index"
end
scope "/admin" do
get "secrets", controller: "secrets", action: "view_secrets"
delete ":user_id", to: "users#destroy"
end
match "photos/:id" => "photos#show", via: :get
match "photos/:id", to: "photos#show", via: :get
match "photos/:id", controller: "photos", action: "show", via: :get
match "photos/:id", to: "photos#show", via: :all
scope controller: "users" do
post "upgrade", action: "start_upgrade"
end
scope module: "enterprise", controller: "billing" do
get "current_billing_cycle"
end
resource :global_config, only: [:show]
namespace :foo do
resources :bar, only: [:index, :show] do
get "show_debug", to: :show_debug
end
end
scope "/users/:user" do
delete "/notifications", to: "users/notifications#destroy", as: :user_destroy_notifications
post "notifications/:notification_id/mark_as_read", to: "users/notifications#mark_as_read"
end
end

View File

@@ -0,0 +1,7 @@
class CommentsController < ApplicationController
def index
end
def show
end
end

View File

@@ -0,0 +1,4 @@
class PhotosController < ApplicationController
def show
end
end

View File

@@ -0,0 +1,10 @@
class PostsController < ApplicationController
def index
end
def show
end
def upvote
end
end

View File

@@ -0,0 +1,6 @@
module Users
class NotificationsController < ApplicationController
def mark_as_read
end
end
end

View File

@@ -3,10 +3,11 @@ edges
| UrlRedirect.rb:14:17:14:22 | call to params : | UrlRedirect.rb:14:17:14:43 | call to fetch |
| UrlRedirect.rb:19:17:19:22 | call to params : | UrlRedirect.rb:19:17:19:37 | call to to_unsafe_hash |
| UrlRedirect.rb:24:31:24:36 | call to params : | UrlRedirect.rb:24:17:24:37 | call to filter_params |
| UrlRedirect.rb:24:31:24:36 | call to params : | UrlRedirect.rb:56:21:56:32 | input_params : |
| UrlRedirect.rb:24:31:24:36 | call to params : | UrlRedirect.rb:63:21:63:32 | input_params : |
| UrlRedirect.rb:34:20:34:25 | call to params : | UrlRedirect.rb:34:20:34:31 | ...[...] : |
| UrlRedirect.rb:34:20:34:31 | ...[...] : | UrlRedirect.rb:34:17:34:37 | "#{...}/foo" |
| UrlRedirect.rb:56:21:56:32 | input_params : | UrlRedirect.rb:57:5:57:29 | call to permit : |
| UrlRedirect.rb:58:17:58:22 | call to params : | UrlRedirect.rb:58:17:58:28 | ...[...] |
| UrlRedirect.rb:63:21:63:32 | input_params : | UrlRedirect.rb:64:5:64:29 | call to permit : |
nodes
| UrlRedirect.rb:4:17:4:22 | call to params | semmle.label | call to params |
| UrlRedirect.rb:9:17:9:22 | call to params : | semmle.label | call to params : |
@@ -20,10 +21,12 @@ nodes
| UrlRedirect.rb:34:17:34:37 | "#{...}/foo" | semmle.label | "#{...}/foo" |
| UrlRedirect.rb:34:20:34:25 | call to params : | semmle.label | call to params : |
| UrlRedirect.rb:34:20:34:31 | ...[...] : | semmle.label | ...[...] : |
| UrlRedirect.rb:56:21:56:32 | input_params : | semmle.label | input_params : |
| UrlRedirect.rb:57:5:57:29 | call to permit : | semmle.label | call to permit : |
| UrlRedirect.rb:58:17:58:22 | call to params : | semmle.label | call to params : |
| UrlRedirect.rb:58:17:58:28 | ...[...] | semmle.label | ...[...] |
| UrlRedirect.rb:63:21:63:32 | input_params : | semmle.label | input_params : |
| UrlRedirect.rb:64:5:64:29 | call to permit : | semmle.label | call to permit : |
subpaths
| UrlRedirect.rb:24:31:24:36 | call to params : | UrlRedirect.rb:56:21:56:32 | input_params : | UrlRedirect.rb:57:5:57:29 | call to permit : | UrlRedirect.rb:24:17:24:37 | call to filter_params |
| UrlRedirect.rb:24:31:24:36 | call to params : | UrlRedirect.rb:63:21:63:32 | input_params : | UrlRedirect.rb:64:5:64:29 | call to permit : | UrlRedirect.rb:24:17:24:37 | call to filter_params |
#select
| UrlRedirect.rb:4:17:4:22 | call to params | UrlRedirect.rb:4:17:4:22 | call to params | UrlRedirect.rb:4:17:4:22 | call to params | Untrusted URL redirection due to $@. | UrlRedirect.rb:4:17:4:22 | call to params | a user-provided value |
| UrlRedirect.rb:9:17:9:28 | ...[...] | UrlRedirect.rb:9:17:9:22 | call to params : | UrlRedirect.rb:9:17:9:28 | ...[...] | Untrusted URL redirection due to $@. | UrlRedirect.rb:9:17:9:22 | call to params | a user-provided value |
@@ -31,3 +34,4 @@ subpaths
| UrlRedirect.rb:19:17:19:37 | call to to_unsafe_hash | UrlRedirect.rb:19:17:19:22 | call to params : | UrlRedirect.rb:19:17:19:37 | call to to_unsafe_hash | Untrusted URL redirection due to $@. | UrlRedirect.rb:19:17:19:22 | call to params | a user-provided value |
| UrlRedirect.rb:24:17:24:37 | call to filter_params | UrlRedirect.rb:24:31:24:36 | call to params : | UrlRedirect.rb:24:17:24:37 | call to filter_params | Untrusted URL redirection due to $@. | UrlRedirect.rb:24:31:24:36 | call to params | a user-provided value |
| UrlRedirect.rb:34:17:34:37 | "#{...}/foo" | UrlRedirect.rb:34:20:34:25 | call to params : | UrlRedirect.rb:34:17:34:37 | "#{...}/foo" | Untrusted URL redirection due to $@. | UrlRedirect.rb:34:20:34:25 | call to params | a user-provided value |
| UrlRedirect.rb:58:17:58:28 | ...[...] | UrlRedirect.rb:58:17:58:22 | call to params : | UrlRedirect.rb:58:17:58:28 | ...[...] | Untrusted URL redirection due to $@. | UrlRedirect.rb:58:17:58:22 | call to params | a user-provided value |

View File

@@ -45,9 +45,16 @@ class UsersController < ActionController::Base
end
# GOOD
# Technically vulnerable, but we assume this is a handler for a POST request,
# Technically vulnerable, this is a handler for a POST request,
# so can't be triggered by following a link.
def create
def create1
redirect_to params[:key]
end
# BAD
# The same as `create1` but this is reachable via a GET request, as configured
# by the routes at the top of this file.
def route9
redirect_to params[:key]
end
@@ -57,3 +64,7 @@ class UsersController < ActionController::Base
input_params.permit(:key)
end
end
Rails.routes.draw do
get "/r9", to: "users#route9"
end