ruby-changes:51752

normal	2018-07-14 11:59:39 +0900 (Sat, 14 Jul 2018)

  New Revision: 63964

  https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=63964

  Log:
    webrick/httpresponse: set_redirect requires a valid URI
    
    Prevents response splitting and HTML injection attacks in
    poorly-written applications which blindly pass along user input
    in redirects.

  Modified files:
    trunk/lib/webrick/httpresponse.rb
    trunk/test/webrick/test_httpresponse.rb
Index: lib/webrick/httpresponse.rb
===================================================================
--- lib/webrick/httpresponse.rb	(revision 63963)
+++ lib/webrick/httpresponse.rb	(revision 63964)
@@ -10,6 +10,7 @@ https://github.com/ruby/ruby/blob/trunk/lib/webrick/httpresponse.rb#L10
 # $IPR: httpresponse.rb,v 1.45 2003/07/11 11:02:25 gotoyuzo Exp $
 
 require 'time'
+require 'uri'
 require 'webrick/httpversion'
 require 'webrick/htmlutils'
 require 'webrick/httputils'
@@ -331,8 +332,9 @@ module WEBrick https://github.com/ruby/ruby/blob/trunk/lib/webrick/httpresponse.rb#L332
     #   res.set_redirect WEBrick::HTTPStatus::TemporaryRedirect
 
     def set_redirect(status, url)
+      url = URI(url).to_s
       @body = "<HTML><A HREF=\"#{url}\">#{url}</A>.</HTML>\n"
-      @header['location'] = url.to_s
+      @header['location'] = url
       raise status
     end
 
Index: test/webrick/test_httpresponse.rb
===================================================================
--- test/webrick/test_httpresponse.rb	(revision 63963)
+++ test/webrick/test_httpresponse.rb	(revision 63964)
@@ -50,6 +50,27 @@ module WEBrick https://github.com/ruby/ruby/blob/trunk/test/webrick/test_httpresponse.rb#L50
       refute_match 'hack', io.string
     end
 
+    def test_set_redirect_response_splitting
+      url = "malicious\r\nCookie: hack"
+      assert_raises(URI::InvalidURIError) do
+        res.set_redirect(WEBrick::HTTPStatus::MultipleChoices, url)
+      end
+    end
+
+    def test_set_redirect_html_injection
+      url = 'http://example.com////?a</a><head></head><body><img src=1></body>'
+      assert_raises(WEBrick::HTTPStatus::MultipleChoices) do
+        res.set_redirect(WEBrick::HTTPStatus::MultipleChoices, url)
+      end
+      res.status = 300
+      io = StringIO.new
+      res.send_response(io)
+      io.rewind
+      res = Net::HTTPResponse.read_new(Net::BufferedIO.new(io))
+      assert_equal '300', res.code
+      refute_match /<img/, io.string
+    end
+
     def test_304_does_not_log_warning
       res.status      = 304
       res.setup_header

--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/