Merge pull request #5456 from brianmario/redirect-sanitization

Strip null bytes from Location header

3 files changed, 22 insertions, 2 deletions

diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb
index b07742e0e1..3ffb7ef426 100644
--- a/actionpack/lib/action_controller/metal/redirecting.rb
+++ b/actionpack/lib/action_controller/metal/redirecting.rb
@@ -93,7 +93,7 @@ module ActionController
           _compute_redirect_to_location options.call
         else
           url_for(options)
-        end.gsub(/[\r\n]/, '')
+        end.gsub(/[\0\r\n]/, '')
       end
   end
 end
diff --git a/actionpack/lib/action_dispatch/testing/assertions/response.rb b/actionpack/lib/action_dispatch/testing/assertions/response.rb
index 094cfbfc76..a5e7a8c715 100644
--- a/actionpack/lib/action_dispatch/testing/assertions/response.rb
+++ b/actionpack/lib/action_dispatch/testing/assertions/response.rb
@@ -83,7 +83,7 @@ module ActionDispatch
             refer
           else
             @controller.url_for(fragment)
-          end.gsub(/[\r\n]/, '')
+          end.gsub(/[\0\r\n]/, '')
         end
     end
   end
diff --git a/actionpack/test/controller/redirect_test.rb b/actionpack/test/controller/redirect_test.rb
index b1d76150f8..a8167a3b69 100644
--- a/actionpack/test/controller/redirect_test.rb
+++ b/actionpack/test/controller/redirect_test.rb
@@ -103,6 +103,14 @@ class RedirectController < ActionController::Base
     redirect_to proc { {:action => "hello_world"} }
   end
 
+  def redirect_with_header_break
+    redirect_to "/lol\r\nwat"
+  end
+
+  def redirect_with_null_bytes
+    redirect_to "\000/lol\r\nwat"
+  end
+
   def rescue_errors(e) raise e end
 
   protected
@@ -120,6 +128,18 @@ class RedirectTest < ActionController::TestCase
     assert_equal "http://test.host/redirect/hello_world", redirect_to_url
   end
 
+  def test_redirect_with_header_break
+    get :redirect_with_header_break
+    assert_response :redirect
+    assert_equal "http://test.host/lolwat", redirect_to_url
+  end
+
+  def test_redirect_with_null_bytes
+    get :redirect_with_header_break
+    assert_response :redirect
+    assert_equal "http://test.host/lolwat", redirect_to_url
+  end
+
   def test_redirect_with_no_status
     get :simple_redirect
     assert_response 302