From 8a81d04d2588d9c7a898473b431a0dabcab39fbd Mon Sep 17 00:00:00 2001

From: usa <usa@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>

Date: Thu, 14 Sep 2017 11:37:47 +0000

Subject: [PATCH] merge revision(s) 59897:

	lib/webrick/log.rb: sanitize any type of logs

	It had failed to sanitize some type of exception messages.  Reported and

	patched by Yusuke Endoh (mame) at https://hackerone.com/reports/223363

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_2_2@59902 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

---

 ChangeLog                     |    7 +++++++

 lib/webrick/httpstatus.rb     |    4 ----

 lib/webrick/log.rb            |    4 ++--

 test/webrick/test_httpauth.rb |   36 ++++++++++++++++++++++++++++++++++++

 4 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/ChangeLog b/ChangeLog

index a4594f678f8c..7561c35eb705 100644

--- a/ChangeLog

+++ b/ChangeLog

@@ -4,6 +4,13 @@

 	  protocol list.

 	  The protocol list from OpenSSL is not null-terminated.

 	  patched by Kazuki Yamaguchi [Bug #11810] [ruby-core:72082]

+

+Thu Sep 14 20:36:54 2017  Yusuke Endoh  <mame@ruby-lang.org>

+

+	lib/webrick/log.rb: sanitize any type of logs

+

+	It had failed to sanitize some type of exception messages.  Reported and

+	patched by Yusuke Endoh (mame) at https://hackerone.com/reports/223363

 Thu Sep 14 20:33:52 2017  Nobuyoshi Nakada  <nobu@ruby-lang.org>

diff --git a/lib/webrick/httpstatus.rb b/lib/webrick/httpstatus.rb

index 7ffda64cf0f9..5dc136f88f70 100644

--- a/lib/webrick/httpstatus.rb

+++ b/lib/webrick/httpstatus.rb

@@ -20,10 +20,6 @@ module HTTPStatus

     ##

     # Root of the HTTP status class hierarchy

     class Status < StandardError

-      def initialize(*args) # :nodoc:

-        args[0] = AccessLog.escape(args[0]) unless args.empty?

-        super(*args)

-      end

       class << self

         attr_reader :code, :reason_phrase # :nodoc:

       end

diff --git a/lib/webrick/log.rb b/lib/webrick/log.rb

index 41cde4a74084..4f069ac0c549 100644

--- a/lib/webrick/log.rb

+++ b/lib/webrick/log.rb

@@ -117,10 +117,10 @@ def debug?; @level >= DEBUG; end

     # * Otherwise it will return +arg+.inspect.

     def format(arg)

       if arg.is_a?(Exception)

-        "#{arg.class}: #{arg.message}\n\t" <<

+        "#{arg.class}: #{AccessLog.escape(arg.message)}\n\t" <<

         arg.backtrace.join("\n\t") << "\n"

       elsif arg.respond_to?(:to_str)

-        arg.to_str

+        AccessLog.escape(arg.to_str)

       else

         arg.inspect

       end

diff --git a/test/webrick/test_httpauth.rb b/test/webrick/test_httpauth.rb

index 27c37f36770b..0aebb7a231c7 100644

--- a/test/webrick/test_httpauth.rb

+++ b/test/webrick/test_httpauth.rb

@@ -79,6 +79,43 @@ def test_basic_auth3

       WEBrick::HTTPAuth::Htpasswd.new(tmpfile.path)

     }

     tmpfile.close(true)

+  end

+

+  def test_bad_username_with_control_characters

+    log_tester = lambda {|log, access_log|

+      assert_equal(2, log.length)

+      assert_match(/ERROR Basic WEBrick's realm: foo\\ebar: the user is not allowed./, log[0])

+      assert_match(/ERROR WEBrick::HTTPStatus::Unauthorized/, log[1])

+    }

+    TestWEBrick.start_httpserver_with_log({}, log_tester) {|server, addr, port, log|

+      realm = "WEBrick's realm"

+      path = "/basic_auth"

+

+      Tempfile.open("test_webrick_auth") {|tmpfile|

+        tmpfile.close

+        tmp_pass = WEBrick::HTTPAuth::Htpasswd.new(tmpfile.path)

+        tmp_pass.set_passwd(realm, "webrick", "supersecretpassword")

+        tmp_pass.set_passwd(realm, "foo", "supersecretpassword")

+        tmp_pass.flush

+

+        htpasswd = WEBrick::HTTPAuth::Htpasswd.new(tmpfile.path)

+        users = []

+        htpasswd.each{|user, pass| users << user }

+        server.mount_proc(path){|req, res|

+          auth = WEBrick::HTTPAuth::BasicAuth.new(

+            :Realm => realm, :UserDB => htpasswd,

+            :Logger => server.logger

+          )

+          auth.authenticate(req, res)

+          res.body = "hoge"

+        }

+        http = Net::HTTP.new(addr, port)

+        g = Net::HTTP::Get.new(path)

+        g.basic_auth("foo\ebar", "passwd")

+        http.request(g){|res| assert_not_equal("hoge", res.body, log.call) }

+        File.unlink tmpfile.path rescue nil

+      }

+    }

   end

   DIGESTRES_ = /

diff --git a/test/webrick/utils.rb b/test/webrick/utils.rb

index e1c2344fb1aa..0e94ad34da71 100644

--- a/test/webrick/utils.rb

+++ b/test/webrick/utils.rb

@@ -54,4 +54,43 @@

   def start_httpproxy(config={}, &block)

     start_server(WEBrick::HTTPProxyServer, config, &block)

   end

+

+  DefaultLogTester = lambda {|log, access_log| assert_equal([], log) }

+

+  def start_server_with_log(klass, config={}, log_tester=DefaultLogTester, &block)

+    log_ary = []

+    access_log_ary = []

+    log = proc { "webrick log start:\n" + (log_ary+access_log_ary).join.gsub(/^/, "  ").chomp + "\nwebrick log end" }

+    server = klass.new({

+      :BindAddress => "127.0.0.1", :Port => 0,

+      :ServerType => Thread,

+      :Logger => WEBrick::Log.new(log_ary, WEBrick::BasicLog::WARN),

+      :AccessLog => [[access_log_ary, ""]]

+    }.update(config))

+    server_thread = server.start

+    server_thread2 = Thread.new {

+      server_thread.join

+      if log_tester

+        log_tester.call(log_ary, access_log_ary)

+      end

+    }

+    addr = server.listeners[0].addr

+    client_thread = Thread.new {

+      begin

+        block.yield([server, addr[3], addr[1], log])

+      ensure

+        server.shutdown

+      end

+    }

+    client_thread.join

+    server_thread2.join

+  end

+

+  def start_httpserver_with_log(config={}, log_tester=DefaultLogTester, &block)

+    start_server_with_log(WEBrick::HTTPServer, config, log_tester, &block)

+  end

+

+  def start_httpproxy_with_log(config={}, log_tester=DefaultLogTester, &block)

+    start_server_with_log(WEBrick::HTTPProxyServer, config, log_tester, &block)

+  end

 end