Merge pull request #30850 from timsly/safe-redirect-back

Add allow_other_host option to redirect_back method

2 files changed, 37 insertions, 7 deletions

diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb
index 5cd8568d8d..b8a80eef31 100644
--- a/actionpack/lib/action_controller/metal/redirecting.rb
+++ b/actionpack/lib/action_controller/metal/redirecting.rb
@@ -79,15 +79,18 @@ module ActionController
     #   redirect_back fallback_location: "/images/screenshot.jpg"
     #   redirect_back fallback_location: posts_url
     #   redirect_back fallback_location: proc { edit_post_url(@post) }
+    #   redirect_back fallback_location: '/', allow_other_host: false
     #
-    # All options that can be passed to <tt>redirect_to</tt> are accepted as
+    # ==== Options
+    # * <tt>:fallback_location</tt> - The default fallback location that will be used on missing `Referer` header.
+    # * <tt>:allow_other_host</tt> - Allows or dissallow redirection to the host that is different to the current host
+    #
+    # 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:, **args)
-      if referer = request.headers["Referer"]
-        redirect_to referer, **args
-      else
-        redirect_to fallback_location, **args
-      end
+    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
     end
 
     def _compute_redirect_to_location(request, options) #:nodoc:
@@ -120,5 +123,11 @@ module ActionController
           302
         end
       end
+
+      def _url_host_allowed?(url)
+        URI(url.to_s).host == request.host
+      rescue ArgumentError, URI::Error
+        false
+      end
   end
 end
diff --git a/actionpack/test/controller/redirect_test.rb b/actionpack/test/controller/redirect_test.rb
index e447b66486..2959dc3e4d 100644
--- a/actionpack/test/controller/redirect_test.rb
+++ b/actionpack/test/controller/redirect_test.rb
@@ -62,6 +62,10 @@ class RedirectController < ActionController::Base
     redirect_back(fallback_location: "/things/stuff", status: 307)
   end
 
+  def safe_redirect_back_with_status
+    redirect_back(fallback_location: "/things/stuff", status: 307, allow_other_host: false)
+  end
+
   def host_redirect
     redirect_to action: "other_host", only_path: false, host: "other.test.host"
   end
@@ -259,6 +263,23 @@ class RedirectTest < ActionController::TestCase
     assert_equal "http://test.host/things/stuff", redirect_to_url
   end
 
+  def test_safe_redirect_back_from_other_host
+    @request.env["HTTP_REFERER"] = "http://another.host/coming/from"
+    get :safe_redirect_back_with_status
+
+    assert_response 307
+    assert_equal "http://test.host/things/stuff", redirect_to_url
+  end
+
+  def test_safe_redirect_back_from_the_same_host
+    referer = "http://test.host/coming/from"
+    @request.env["HTTP_REFERER"] = referer
+    get :safe_redirect_back_with_status
+
+    assert_response 307
+    assert_equal referer, redirect_to_url
+  end
+
   def test_redirect_to_record
     with_routing do |set|
       set.draw do