kotovalexarian-likes-github
/
rails--rails
mirror of https://github.com/rails/rails.git
1
0
Fork 0
Code Releases Activity

Ensure external redirects are explicitly allowed 

Add `fallback_location` and `allow_other_host` options to `redirect_to`.
This commit is contained in:
Gannon McGibbon 2018-11-08 14:45:06 -05:00
parent 9e34df0003
commit 9dde7d8de0
7 changed files with 87 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
actionpack/CHANGELOG.md
View File

@ -1,3 +1,9 @@
* Ensure external redirects are explicitly allowed
Add `fallback_location` and `allow_other_host` options to `redirect_to`.
*Gannon McGibbon*
* Introduce ActionDispatch::HostAuthorization
This is a new middleware that guards against DNS rebinding attacks by

8
actionpack/lib/action_controller/metal/flash.rb
View File

@ -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

5
actionpack/lib/action_controller/metal/force_ssl.rb
View File

@ -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)

43
actionpack/lib/action_controller/metal/redirecting.rb
View File

@ -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

6
actionpack/test/controller/action_pack_assertions_test.rb
View File

@ -28,13 +28,13 @@ class ActionPackAssertionsController < ActionController::Base
def redirect_to_path() redirect_to "/some/path" end
def redirect_invalid_external_route() redirect_to "ht_tp://www.rubyonrails.org" end
def redirect_invalid_external_route() redirect_to "ht_tp://www.rubyonrails.org", allow_other_host: true end
def redirect_to_named_route() redirect_to route_one_url end
def redirect_external() redirect_to "http://www.rubyonrails.org"; end
def redirect_external() redirect_to "http://www.rubyonrails.org", allow_other_host: true; end
def redirect_external_protocol_relative() redirect_to "//www.rubyonrails.org"; end
def redirect_external_protocol_relative() redirect_to "//www.rubyonrails.org", allow_other_host: true; end
def response404() head "404 AWOL" end

4
actionpack/test/controller/log_subscriber_test.rb
View File

@ -25,11 +25,11 @@ module Another
end
def redirector
redirect_to "http://foo.bar/"
redirect_to "http://foo.bar/", allow_other_host: true
end
def filterable_redirector
redirect_to "http://secret.foo.bar/"
redirect_to "http://secret.foo.bar/", allow_other_host: true
end
def data_sender

44
actionpack/test/controller/redirect_test.rb
View File

@ -49,11 +49,11 @@ class RedirectController < ActionController::Base
end
def url_redirect_with_status
redirect_to("http://www.example.com", status: :moved_permanently)
redirect_to("http://www.example.com", status: :moved_permanently, allow_other_host: true)
end
def url_redirect_with_status_hash
redirect_to("http://www.example.com", status: 301)
redirect_to("http://www.example.com", status: 301, allow_other_host: true)
end
def relative_url_redirect_with_status
@ -81,19 +81,27 @@ class RedirectController < ActionController::Base
end
def redirect_to_url
redirect_to "http://www.rubyonrails.org/", allow_other_host: true
end
def redirect_to_unsafe_url
redirect_to "http://www.rubyonrails.org/"
end
def redirect_to_relative_unsafe_url
redirect_to ".br"
end
def redirect_to_url_with_unescaped_query_string
redirect_to "http://example.com/query?status=new"
redirect_to "http://example.com/query?status=new", allow_other_host: true
end
def redirect_to_url_with_complex_scheme
redirect_to "x-test+scheme.complex:redirect"
redirect_to "x-test+scheme.complex:redirect", allow_other_host: true
end
def redirect_to_url_with_network_path_reference
redirect_to "//www.rubyonrails.org/"
redirect_to "//www.rubyonrails.org/", allow_other_host: true
end
def redirect_to_existing_record
@ -113,12 +121,12 @@ class RedirectController < ActionController::Base
end
def redirect_to_with_block
redirect_to proc { "http://www.rubyonrails.org/" }
redirect_to proc { "http://www.rubyonrails.org/" }, allow_other_host: true
end
def redirect_to_with_block_and_assigns
@url = "http://www.rubyonrails.org/"
redirect_to proc { @url }
redirect_to proc { @url }, allow_other_host: true
end
def redirect_to_with_block_and_options
@ -245,6 +253,28 @@ class RedirectTest < ActionController::TestCase
assert_redirected_to "http://www.rubyonrails.org/"
end
def test_redirect_to_unsafe_url
error = assert_raises(ArgumentError) do
get :redirect_to_unsafe_url
end
assert_equal <<~MSG.squish, error.message
Unsafe redirect \"http://www.rubyonrails.org/\",
use :fallback_location to specify a fallback or
:allow_other_host to redirect anyway.
MSG
end
def test_redirect_to_relative_unsafe_url
error = assert_raises(ArgumentError) do
get :redirect_to_relative_unsafe_url
end
assert_equal <<~MSG.squish, error.message
Unsafe redirect \"http://test.host.br\",
use :fallback_location to specify a fallback or
:allow_other_host to redirect anyway.
MSG
end
def test_redirect_to_url_with_unescaped_query_string
get :redirect_to_url_with_unescaped_query_string
assert_response :redirect