diff options
| author | Gannon McGibbon <gannon.mcgibbon@gmail.com> | 2018-11-08 14:45:06 -0500 |
|---|---|---|
| committer | Gannon McGibbon <gannon.mcgibbon@gmail.com> | 2019-01-17 13:28:14 -0500 |
| commit | 9dde7d8de047b55ec636c4c7cba89ec95324d492 (patch) | |
| tree | 5afafc7ecef072ad779664827355cb0e91cc3768 /actionpack/lib/action_controller | |
| parent | 9e34df00039d63b5672315419e76f06f80ef3dc4 (diff) | |
| download | rails-9dde7d8de047b55ec636c4c7cba89ec95324d492.tar.gz rails-9dde7d8de047b55ec636c4c7cba89ec95324d492.tar.bz2 rails-9dde7d8de047b55ec636c4c7cba89ec95324d492.zip | |
Ensure external redirects are explicitly allowed
Add `fallback_location` and `allow_other_host` options to `redirect_to`.
Diffstat (limited to 'actionpack/lib/action_controller')
| -rw-r--r-- | actionpack/lib/action_controller/metal/flash.rb | 8 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/metal/force_ssl.rb | 5 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/metal/redirecting.rb | 43 |
3 files changed, 39 insertions, 17 deletions
diff --git a/actionpack/lib/action_controller/metal/flash.rb b/actionpack/lib/action_controller/metal/flash.rb index 380f2e9591..a4861dc2c0 100644 --- a/actionpack/lib/action_controller/metal/flash.rb +++ b/actionpack/lib/action_controller/metal/flash.rb @@ -44,18 +44,18 @@ module ActionController #:nodoc: end private - def redirect_to(options = {}, response_status_and_flash = {}) #:doc: + def redirect_to(options = {}, response_options_and_flash = {}) #:doc: self.class._flash_types.each do |flash_type| - if type = response_status_and_flash.delete(flash_type) + if type = response_options_and_flash.delete(flash_type) flash[flash_type] = type end end - if other_flashes = response_status_and_flash.delete(:flash) + if other_flashes = response_options_and_flash.delete(:flash) flash.update(other_flashes) end - super(options, response_status_and_flash) + super(options, response_options_and_flash) end end end diff --git a/actionpack/lib/action_controller/metal/force_ssl.rb b/actionpack/lib/action_controller/metal/force_ssl.rb index 26e6f72b66..205f84ae36 100644 --- a/actionpack/lib/action_controller/metal/force_ssl.rb +++ b/actionpack/lib/action_controller/metal/force_ssl.rb @@ -13,7 +13,7 @@ module ActionController ACTION_OPTIONS = [:only, :except, :if, :unless] URL_OPTIONS = [:protocol, :host, :domain, :subdomain, :port, :path] - REDIRECT_OPTIONS = [:status, :flash, :alert, :notice] + REDIRECT_OPTIONS = [:status, :flash, :alert, :notice, :allow_other_host] module ClassMethods # :nodoc: def force_ssl(options = {}) @@ -40,7 +40,8 @@ module ActionController protocol: "https://", host: request.host, path: request.fullpath, - status: :moved_permanently + status: :moved_permanently, + allow_other_host: true, } if host_or_options.is_a?(Hash) diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb index 2804a06a58..8bd003f5ed 100644 --- a/actionpack/lib/action_controller/metal/redirecting.rb +++ b/actionpack/lib/action_controller/metal/redirecting.rb @@ -55,12 +55,12 @@ module ActionController # Statements after +redirect_to+ in our controller get executed, so +redirect_to+ doesn't stop the execution of the function. # To terminate the execution of the function immediately after the +redirect_to+, use return. # redirect_to post_url(@post) and return - def redirect_to(options = {}, response_status = {}) + def redirect_to(options = {}, response_options = {}) raise ActionControllerError.new("Cannot redirect to nil!") unless options raise AbstractController::DoubleRenderError if response_body - self.status = _extract_redirect_to_status(options, response_status) - self.location = _compute_redirect_to_location(request, options) + self.status = _extract_redirect_to_status(options, response_options) + self.location = _compute_safe_redirect_to_location(request, options, response_options) self.response_body = "<html><body>You are being <a href=\"#{ERB::Util.unwrapped_html_escape(response.location)}\">redirected</a>.</body></html>" end @@ -88,9 +88,13 @@ module ActionController # All other options that can be passed to <tt>redirect_to</tt> are accepted as # options and the behavior is identical. def redirect_back(fallback_location:, allow_other_host: true, **args) - referer = request.headers["Referer"] - redirect_to_referer = referer && (allow_other_host || _url_host_allowed?(referer)) - redirect_to redirect_to_referer ? referer : fallback_location, **args + referer = request.headers.fetch("Referer", fallback_location) + response_options = { + fallback_location: fallback_location, + allow_other_host: allow_other_host, + **args, + } + redirect_to referer, response_options end def _compute_redirect_to_location(request, options) #:nodoc: @@ -114,18 +118,35 @@ module ActionController public :_compute_redirect_to_location private - def _extract_redirect_to_status(options, response_status) + def _compute_safe_redirect_to_location(request, options, response_options) + location = _compute_redirect_to_location(request, options) + location_options = options.is_a?(Hash) ? options : {} + if response_options[:allow_other_host] || _url_host_allowed?(location, location_options) + location + else + fallback_location = response_options.fetch(:fallback_location) do + raise ArgumentError, <<~MSG.squish + Unsafe redirect #{location.inspect}, + use :fallback_location to specify a fallback + or :allow_other_host to redirect anyway. + MSG + end + _compute_redirect_to_location(request, fallback_location) + end + end + + def _extract_redirect_to_status(options, response_options) if options.is_a?(Hash) && options.key?(:status) Rack::Utils.status_code(options.delete(:status)) - elsif response_status.key?(:status) - Rack::Utils.status_code(response_status[:status]) + elsif response_options.key?(:status) + Rack::Utils.status_code(response_options[:status]) else 302 end end - def _url_host_allowed?(url) - URI(url.to_s).host == request.host + def _url_host_allowed?(url, options = {}) + URI(url.to_s).host.in?([request.host, options[:host]]) rescue ArgumentError, URI::Error false end |