hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-30 19:26:02 +02:00
Code Issues Packages Projects Releases Wiki Activity

Ruby: Model redirect_back and redirect_back_or_to

Browse Source 
These are ActionController methods that redirect to the HTTP Referer,
falling back to the given location if there is no Referer.
This commit is contained in:
Harry Maclean
2022-06-16 15:33:01 +12:00
parent a298f5eb5e
commit 7dfab371f6
7 changed files with 84 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
ruby/ql/lib/codeql/ruby/frameworks/ActionController.qll
View File

@@ -180,18 +180,31 @@ private class ActionControllerHtmlEscapeCall extends HtmlEscapeCall {
* specific URL/path or to a different action in this controller.
*/
class RedirectToCall extends ActionControllerContextCall {
RedirectToCall() { this.getMethodName() = "redirect_to" }
RedirectToCall() {
this.getMethodName() = ["redirect_to", "redirect_back", "redirect_back_or_to"]
}
/** Gets the `Expr` representing the URL to redirect to, if any */
Expr getRedirectUrl() { result = this.getArgument(0) }
Expr getRedirectUrl() {
this.getMethodName() = "redirect_back" and result = this.getKeywordArgument("fallback_location")
or
this.getMethodName() = ["redirect_to", "redirect_back_or_to"] and result = this.getArgument(0)
}
/** Gets the `ActionControllerActionMethod` to redirect to, if any */
ActionControllerActionMethod getRedirectActionMethod() {
exists(string methodName |
this.getKeywordArgument("action").getConstantValue().isStringlikeValue(methodName) and
methodName = result.getName() and
result.getEnclosingModule() = this.getControllerClass()
)
this.getKeywordArgument("action").getConstantValue().isStringlikeValue(result.getName()) and
result.getEnclosingModule() = this.getControllerClass()
}
/**
* Holds if this method call allows a redirect to an external host.
*/
predicate allowsExternalRedirect() {
// Unless the option allow_other_host is explicitly set to false, assume that external redirects are allowed.
// TODO: Take into account `config.action_controller.raise_on_open_redirects`.
// TODO: Take into account that this option defaults to false in Rails 7.
not this.getKeywordArgument("allow_other_host").getConstantValue().isBoolean(false)
}
}

4
ruby/ql/lib/codeql/ruby/security/UrlRedirectCustomizations.qll
View File

@@ -69,7 +69,9 @@ module UrlRedirect {
// 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).*")
)
) and
// If this redirect is an ActionController method call, it is only vulnerable if it allows external redirects.
forall(RedirectToCall c | c = e.asExpr().getExpr() | c.allowsExternalRedirect())
)
}
}

14
ruby/ql/test/library-tests/frameworks/ActionController.expected
View File

@@ -3,7 +3,7 @@ actionControllerControllerClasses
| ActiveRecord.rb:41:1:64:3 | BarController |
| ActiveRecord.rb:66:1:70: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/foo/bars_controller.rb:3:1:39: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 |
@@ -17,6 +17,8 @@ actionControllerActionMethods
| 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/foo/bars_controller.rb:26:3:28:5 | go_back |
| app/controllers/foo/bars_controller.rb:30:3:32:5 | go_back_2 |
| 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 |
@@ -66,10 +68,12 @@ cookiesSources
| app/controllers/foo/bars_controller.rb:10:27:10:33 | call to cookies |
redirectToCalls
| app/controllers/foo/bars_controller.rb:17:5:17:30 | call to redirect_to |
| app/controllers/foo/bars_controller.rb:27:5:27:39 | call to redirect_back_or_to |
| app/controllers/foo/bars_controller.rb:31:5:31:56 | call to redirect_back |
actionControllerHelperMethods
getAssociatedControllerClasses
| app/controllers/foo/bars_controller.rb:3:1:31:3 | BarsController | app/views/foo/bars/_widget.html.erb:0:0:0:0 | app/views/foo/bars/_widget.html.erb |
| app/controllers/foo/bars_controller.rb:3:1:31:3 | BarsController | app/views/foo/bars/show.html.erb:0:0:0:0 | app/views/foo/bars/show.html.erb |
| app/controllers/foo/bars_controller.rb:3:1:39:3 | BarsController | app/views/foo/bars/_widget.html.erb:0:0:0:0 | app/views/foo/bars/_widget.html.erb |
| app/controllers/foo/bars_controller.rb:3:1:39:3 | BarsController | app/views/foo/bars/show.html.erb:0:0:0:0 | app/views/foo/bars/show.html.erb |
controllerTemplateFiles
| app/controllers/foo/bars_controller.rb:3:1:31:3 | BarsController | app/views/foo/bars/_widget.html.erb:0:0:0:0 | app/views/foo/bars/_widget.html.erb |
| app/controllers/foo/bars_controller.rb:3:1:31:3 | BarsController | app/views/foo/bars/show.html.erb:0:0:0:0 | app/views/foo/bars/show.html.erb |
| app/controllers/foo/bars_controller.rb:3:1:39:3 | BarsController | app/views/foo/bars/_widget.html.erb:0:0:0:0 | app/views/foo/bars/_widget.html.erb |
| app/controllers/foo/bars_controller.rb:3:1:39:3 | BarsController | app/views/foo/bars/show.html.erb:0:0:0:0 | app/views/foo/bars/show.html.erb |

2
ruby/ql/test/library-tests/frameworks/ActionView.expected
View File

@@ -14,7 +14,7 @@ rawCalls
renderCalls
| app/controllers/foo/bars_controller.rb:6:5:6:37 | call to render |
| app/controllers/foo/bars_controller.rb:23:5:23:76 | call to render |
| app/controllers/foo/bars_controller.rb:29:5:29:17 | call to render |
| app/controllers/foo/bars_controller.rb:37:5:37:17 | call to render |
| app/views/foo/bars/show.html.erb:31:5:31:89 | call to render |
renderToCalls
| app/controllers/foo/bars_controller.rb:15:16:15:97 | call to render_to_string |

8
ruby/ql/test/library-tests/frameworks/app/controllers/foo/bars_controller.rb
View File

@@ -23,6 +23,14 @@ class BarsController < ApplicationController
render "foo/bars/show", locals: { display_text: dt, safe_text: "hello" }
end
def go_back
redirect_back_or_to action: "index"
end
def go_back_2
redirect_back fallback_location: { action: "index" }
end
private
def unreachable_action

22
ruby/ql/test/query-tests/security/cwe-601/UrlRedirect.expected
View File

@@ -3,11 +3,14 @@ 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:63:21:63:32 | input_params : |
| UrlRedirect.rb:24:31:24:36 | call to params : | UrlRedirect.rb:88:21:88: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: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 : |
| UrlRedirect.rb:63:38:63:43 | call to params : | UrlRedirect.rb:63:38:63:49 | ...[...] |
| UrlRedirect.rb:68:38:68:43 | call to params : | UrlRedirect.rb:68:38:68:49 | ...[...] |
| UrlRedirect.rb:73:25:73:30 | call to params : | UrlRedirect.rb:73:25:73:36 | ...[...] |
| UrlRedirect.rb:88:21:88:32 | input_params : | UrlRedirect.rb:89:5:89: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 : |
@@ -23,10 +26,16 @@ nodes
| UrlRedirect.rb:34:20:34:31 | ...[...] : | semmle.label | ...[...] : |
| 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 : |
| UrlRedirect.rb:63:38:63:43 | call to params : | semmle.label | call to params : |
| UrlRedirect.rb:63:38:63:49 | ...[...] | semmle.label | ...[...] |
| UrlRedirect.rb:68:38:68:43 | call to params : | semmle.label | call to params : |
| UrlRedirect.rb:68:38:68:49 | ...[...] | semmle.label | ...[...] |
| UrlRedirect.rb:73:25:73:30 | call to params : | semmle.label | call to params : |
| UrlRedirect.rb:73:25:73:36 | ...[...] | semmle.label | ...[...] |
| UrlRedirect.rb:88:21:88:32 | input_params : | semmle.label | input_params : |
| UrlRedirect.rb:89:5:89:29 | call to permit : | semmle.label | call to permit : |
subpaths
| 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 |
| UrlRedirect.rb:24:31:24:36 | call to params : | UrlRedirect.rb:88:21:88:32 | input_params : | UrlRedirect.rb:89:5:89: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 |
@@ -35,3 +44,6 @@ subpaths
| 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 |
| UrlRedirect.rb:63:38:63:49 | ...[...] | UrlRedirect.rb:63:38:63:43 | call to params : | UrlRedirect.rb:63:38:63:49 | ...[...] | Untrusted URL redirection due to $@. | UrlRedirect.rb:63:38:63:43 | call to params | a user-provided value |
| UrlRedirect.rb:68:38:68:49 | ...[...] | UrlRedirect.rb:68:38:68:43 | call to params : | UrlRedirect.rb:68:38:68:49 | ...[...] | Untrusted URL redirection due to $@. | UrlRedirect.rb:68:38:68:43 | call to params | a user-provided value |
| UrlRedirect.rb:73:25:73:36 | ...[...] | UrlRedirect.rb:73:25:73:30 | call to params : | UrlRedirect.rb:73:25:73:36 | ...[...] | Untrusted URL redirection due to $@. | UrlRedirect.rb:73:25:73:30 | call to params | a user-provided value |

27
ruby/ql/test/query-tests/security/cwe-601/UrlRedirect.rb
View File

@@ -53,11 +53,36 @@ class UsersController < ActionController::Base
# BAD
# The same as `create1` but this is reachable via a GET request, as configured
# by the routes at the top of this file.
# by the routes at the bottom of this file.
def route9
redirect_to params[:key]
end
# BAD
def route10
redirect_back fallback_location: params[:key]
end
# BAD
def route11
redirect_back fallback_location: params[:key], allow_other_host: true
end
# BAD
def route12
redirect_back_or_to params[:key]
end
# GOOD
def route13
redirect_back fallback_location: params[:key], allow_other_host: false
end
# GOOD
def route14
redirect_back_or_to params[:key], allow_other_host: false
end
private
def filter_params(input_params)