From 9dde7d8de047b55ec636c4c7cba89ec95324d492 Mon Sep 17 00:00:00 2001
From: Gannon McGibbon <gannon.mcgibbon@gmail.com>
Date: Thu, 8 Nov 2018 14:45:06 -0500
Subject: Ensure external redirects are explicitly allowed

Add `fallback_location` and `allow_other_host` options to `redirect_to`.
---
 .../test/controller/action_pack_assertions_test.rb |  6 +--
 actionpack/test/controller/log_subscriber_test.rb  |  4 +-
 actionpack/test/controller/redirect_test.rb        | 44 ++++++++++++++++++----
 3 files changed, 42 insertions(+), 12 deletions(-)

(limited to 'actionpack/test')

diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb
index ecb8c37e6b..c7aae034dd 100644
--- a/actionpack/test/controller/action_pack_assertions_test.rb
+++ b/actionpack/test/controller/action_pack_assertions_test.rb
@@ -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
 
diff --git a/actionpack/test/controller/log_subscriber_test.rb b/actionpack/test/controller/log_subscriber_test.rb
index 0562c16284..cbebc6b59c 100644
--- a/actionpack/test/controller/log_subscriber_test.rb
+++ b/actionpack/test/controller/log_subscriber_test.rb
@@ -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
diff --git a/actionpack/test/controller/redirect_test.rb b/actionpack/test/controller/redirect_test.rb
index 998498e1b2..945d2275c0 100644
--- a/actionpack/test/controller/redirect_test.rb
+++ b/actionpack/test/controller/redirect_test.rb
@@ -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
-- 
cgit v1.2.3