Strip null bytes from Location header #5456

Merged
merged 1 commit into from Mar 15, 2012

2 participants

@brianmario

This strips null bytes off of the Location header in addition to \r\n which are already being stripped.

This is in response to a recent nasty security vulnerability in nginx. I figure it's probably easier for people to get this patch in their Rails applications than it is for them to get nginx upgraded in their infrastructure - which may or may not even be managed by them.

I'd like to get this into all supported Rails versions, not sure if this patch will apply cleanly to 3.1 or 3.0 - let me know if not and I'll send some others.

@brianmario brianmario strip null bytes from Location header as well
add tests for stripping \r\n chars since that's already happening
cfcdd33
@tenderlove tenderlove merged commit a7dee1a into rails:master Mar 15, 2012
@carlosantoniodasilva carlosantoniodasilva commented on the diff Mar 15, 2012
actionpack/test/controller/redirect_test.rb
@@ -120,6 +128,18 @@ def test_simple_redirect
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
@carlosantoniodasilva
Ruby on Rails member

Hey, I think this should read redirect_with_null_bytes instead, not?

@brianmario
brianmario added a line comment Mar 15, 2012

gah, yep :(

@tenderlove
Ruby on Rails member
tenderlove added a line comment Mar 15, 2012

another PR plz? ❤️

@brianmario
brianmario added a line comment Mar 15, 2012

on it

@tenderlove
Ruby on Rails member
tenderlove added a line comment Mar 15, 2012

❤️❤️❤️❤️❤️❤️❤️❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment