* lib/webrick/httpservlet/filehandler.rb: should normalize path

  name in path_info to prevent script disclosure vulnerability on
  DOSISH filesystems. (fix: CVE-2008-1891)
  Note: NTFS/FAT filesystem should not be published by the platforms
  other than Windows. Pathname interpretation (including short
  filename) is less than perfect.

* lib/webrick/httpservlet/abstract.rb
  (WEBrick::HTTPServlet::AbstracServlet#redirect_to_directory_uri):
  should escape the value of Location: header.

* lib/webrick/httpservlet/cgi_runner.rb: accept interpreter
  command line arguments.


git-svn-id: http://svn.ruby-lang.org/repos/ruby/branches/ruby_1_8_6@16495 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

10 files changed, 191 insertions, 35 deletions

diff --git a/ChangeLog b/ChangeLog
index ccce9f4f7..e7070f192 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,19 @@
+Wed May 21 01:32:56 2008  GOTOU Yuuzou  <gotoyuzo@notwork.org>
+
+	* lib/webrick/httpservlet/filehandler.rb: should normalize path
+	  name in path_info to prevent script disclosure vulnerability on
+	  DOSISH filesystems. (fix: CVE-2008-1891)
+	  Note: NTFS/FAT filesystem should not be published by the platforms
+	  other than Windows. Pathname interpretation (including short
+	  filename) is less than perfect.
+
+	* lib/webrick/httpservlet/abstract.rb
+	  (WEBrick::HTTPServlet::AbstracServlet#redirect_to_directory_uri):
+	  should escape the value of Location: header.
+
+	* lib/webrick/httpservlet/cgi_runner.rb: accept interpreter
+	  command line arguments.
+
 Sun May 18 01:57:44 2008  Nobuyoshi Nakada  <nobu@ruby-lang.org>
 
 	* file.c (isdirsep): backslash is valid path separator on cygwin too.
diff --git a/lib/webrick/httpservlet/abstract.rb b/lib/webrick/httpservlet/abstract.rb
index 03861e8fc..5375c4622 100644
--- a/lib/webrick/httpservlet/abstract.rb
+++ b/lib/webrick/httpservlet/abstract.rb
@@ -58,7 +58,7 @@ module WEBrick
 
       def redirect_to_directory_uri(req, res)
         if req.path[-1] != ?/
-          location = req.path + "/"
+          location = WEBrick::HTTPUtils.escape_path(req.path + "/")
           if req.query_string && req.query_string.size > 0
             location << "?" << req.query_string
           end
diff --git a/lib/webrick/httpservlet/cgi_runner.rb b/lib/webrick/httpservlet/cgi_runner.rb
index 1069a68d5..006abd458 100644
--- a/lib/webrick/httpservlet/cgi_runner.rb
+++ b/lib/webrick/httpservlet/cgi_runner.rb
@@ -39,7 +39,9 @@ dir = File::dirname(ENV["SCRIPT_FILENAME"])
 Dir::chdir dir
 
 if interpreter = ARGV[0]
-  exec(interpreter, ENV["SCRIPT_FILENAME"])
+  argv = ARGV.dup
+  argv << ENV["SCRIPT_FILENAME"]
+  exec(*argv)
   # NOTREACHED
 end
 exec ENV["SCRIPT_FILENAME"]
diff --git a/lib/webrick/httpservlet/filehandler.rb b/lib/webrick/httpservlet/filehandler.rb
index c8278b7b8..24f59d714 100644
--- a/lib/webrick/httpservlet/filehandler.rb
+++ b/lib/webrick/httpservlet/filehandler.rb
@@ -199,26 +199,38 @@ module WEBrick
 
       private
 
+      def trailing_pathsep?(path)
+        # check for trailing path separator:
+        #   File.dirname("/aaaa/bbbb/")      #=> "/aaaa")
+        #   File.dirname("/aaaa/bbbb/x")     #=> "/aaaa/bbbb")
+        #   File.dirname("/aaaa/bbbb")       #=> "/aaaa")
+        #   File.dirname("/aaaa/bbbbx")      #=> "/aaaa")
+        return File.dirname(path) != File.dirname(path+"x")
+      end
+
       def prevent_directory_traversal(req, res)
-        # Preventing directory traversal on DOSISH platforms;
+        # Preventing directory traversal on Windows platforms;
         # Backslashes (0x5c) in path_info are not interpreted as special
         # character in URI notation. So the value of path_info should be
         # normalize before accessing to the filesystem.
-        if File::ALT_SEPARATOR
+
+        if trailing_pathsep?(req.path_info)
           # File.expand_path removes the trailing path separator.
           # Adding a character is a workaround to save it.
           #  File.expand_path("/aaa/")        #=> "/aaa"
           #  File.expand_path("/aaa/" + "x")  #=> "/aaa/x"
           expanded = File.expand_path(req.path_info + "x")
-          expanded[-1, 1] = ""  # remove trailing "x"
-          req.path_info = expanded
+          expanded.chop!  # remove trailing "x"
+        else
+          expanded = File.expand_path(req.path_info)
         end
+        req.path_info = expanded
       end
 
       def exec_handler(req, res)
         raise HTTPStatus::NotFound, "`#{req.path}' not found" unless @root
         if set_filename(req, res)
-          handler = get_handler(req)
+          handler = get_handler(req, res)
           call_callback(:HandlerCallback, req, res)
           h = handler.get_instance(@config, res.filename)
           h.service(req, res)
@@ -228,9 +240,13 @@ module WEBrick
         return false
       end
 
-      def get_handler(req)
-        suffix1 = (/\.(\w+)$/ =~ req.script_name) && $1.downcase
-        suffix2 = (/\.(\w+)\.[\w\-]+$/ =~ req.script_name) && $1.downcase
+      def get_handler(req, res)
+        suffix1 = (/\.(\w+)\z/ =~ res.filename) && $1.downcase
+        if /\.(\w+)\.([\w\-]+)\z/ =~ res.filename
+          if @options[:AcceptableLanguages].include?($2.downcase)
+            suffix2 = $1.downcase
+          end
+        end
         handler_table = @options[:HandlerTable]
         return handler_table[suffix1] || handler_table[suffix2] ||
                HandlerTable[suffix1] || HandlerTable[suffix2] ||
@@ -243,15 +259,13 @@ module WEBrick
 
         path_info.unshift("")  # dummy for checking @root dir
         while base = path_info.first
-          check_filename(req, res, base)
           break if base == "/"
-          break unless File.directory?(res.filename + base)
+          break unless File.directory?(File.expand_path(res.filename + base))
           shift_path_info(req, res, path_info)
           call_callback(:DirectoryCallback, req, res)
         end
 
         if base = path_info.first
-          check_filename(req, res, base)
           if base == "/"
             if file = search_index_file(req, res)
               shift_path_info(req, res, path_info, file)
@@ -272,12 +286,10 @@ module WEBrick
       end
 
       def check_filename(req, res, name)
-        @options[:NondisclosureName].each{|pattern|
-          if File.fnmatch("/#{pattern}", name, File::FNM_CASEFOLD)
-            @logger.warn("the request refers nondisclosure name `#{name}'.")
-            raise HTTPStatus::NotFound, "`#{req.path}' not found."
-          end
-        }
+        if nondisclosure_name?(name) || windows_ambiguous_name?(name)
+          @logger.warn("the request refers nondisclosure name `#{name}'.")
+          raise HTTPStatus::NotFound, "`#{req.path}' not found."
+        end
       end
 
       def shift_path_info(req, res, path_info, base=nil)
@@ -285,7 +297,8 @@ module WEBrick
         base = base || tmp
         req.path_info = path_info.join
         req.script_name << base
-        res.filename << base
+        res.filename = File.expand_path(res.filename + base)
+        check_filename(req, res, File.basename(res.filename))
       end
 
       def search_index_file(req, res)
@@ -325,6 +338,12 @@ module WEBrick
         end
       end
 
+      def windows_ambiguous_name?(name)
+        return true if /[. ]+\z/ =~ name
+        return true if /::\$DATA\z/ =~ name
+        return false
+      end
+
       def nondisclosure_name?(name)
         @options[:NondisclosureName].each{|pattern|
           if File.fnmatch(pattern, name, File::FNM_CASEFOLD)
@@ -343,7 +362,8 @@ module WEBrick
         list = Dir::entries(local_path).collect{|name|
           next if name == "." || name == ".."
           next if nondisclosure_name?(name)
-          st = (File::stat(local_path + name) rescue nil)
+          next if windows_ambiguous_name?(name)
+          st = (File::stat(File.join(local_path, name)) rescue nil)
           if st.nil?
             [ name, nil, -1 ]
           elsif st.directory?
@@ -383,7 +403,7 @@ module WEBrick
         res.body << "<A HREF=\"?S=#{d1}\">Size</A>\n"
         res.body << "<HR>\n"
        
-        list.unshift [ "..", File::mtime(local_path+".."), -1 ]
+        list.unshift [ "..", File::mtime(local_path+"/.."), -1 ]
         list.each{ |name, time, size|
           if name == ".."
             dname = "Parent Directory"
diff --git a/test/webrick/.htaccess b/test/webrick/.htaccess
new file mode 100644
index 000000000..69d4659b9
--- /dev/null
+++ b/test/webrick/.htaccess
@@ -0,0 +1 @@
+this file should not be published.
diff --git a/test/webrick/test_cgi.rb b/test/webrick/test_cgi.rb
index 7e3f4ee5f..b39fe2661 100644
--- a/test/webrick/test_cgi.rb
+++ b/test/webrick/test_cgi.rb
@@ -1,20 +1,13 @@
 require "webrick"
 require File.join(File.dirname(__FILE__), "utils.rb")
 require "test/unit"
-begin
-  loadpath = $:.dup
-  $:.replace($: | [File.expand_path("../ruby", File.dirname(__FILE__))])
-  require 'envutil'
-ensure
-  $:.replace(loadpath)
-end
 
 class TestWEBrickCGI < Test::Unit::TestCase
   def test_cgi
     accepted = started = stopped = 0
     requested0 = requested1 = 0
     config = {
-      :CGIInterpreter => EnvUtil.rubybin,
+      :CGIInterpreter => TestWEBrick::RubyBin,
       :DocumentRoot => File.dirname(__FILE__),
       :DirectoryIndex => ["webrick.cgi"],
     }
diff --git a/test/webrick/test_filehandler.rb b/test/webrick/test_filehandler.rb
index e1299a458..2c7d97fa7 100644
--- a/test/webrick/test_filehandler.rb
+++ b/test/webrick/test_filehandler.rb
@@ -9,6 +9,10 @@ class WEBrick::TestFileHandler < Test::Unit::TestCase
     klass.new(WEBrick::Config::HTTP, filename)
   end
 
+  def windows?
+    File.directory?("\\")
+  end
+
   def get_res_body(res)
     return res.body.read rescue res.body
   end
@@ -115,10 +119,82 @@ class WEBrick::TestFileHandler < Test::Unit::TestCase
       http = Net::HTTP.new(addr, port)
       req = Net::HTTP::Get.new("/../../")
       http.request(req){|res| assert_equal("400", res.code) }
-      req = Net::HTTP::Get.new(
-        "/..%5c..%5c..%5c..%5c..%5c..%5c..%5c..%5c..%5c..%5c..%5c..%5cboot.ini"
-      )
+      req = Net::HTTP::Get.new("/..%5c../#{File.basename(__FILE__)}")
+      http.request(req){|res| assert_equal(windows? ? "200" : "404", res.code) }
+      req = Net::HTTP::Get.new("/..%5c..%5cruby.c")
       http.request(req){|res| assert_equal("404", res.code) }
     end
   end
+
+  def test_unwise_in_path
+    if windows?
+      config = { :DocumentRoot => File.dirname(__FILE__), }
+      this_file = File.basename(__FILE__)
+      TestWEBrick.start_httpserver(config) do |server, addr, port|
+        http = Net::HTTP.new(addr, port)
+        req = Net::HTTP::Get.new("/..%5c..")
+        http.request(req){|res| assert_equal("301", res.code) }
+      end
+    end
+  end
+
+  def test_short_filename
+    config = {
+      :CGIInterpreter => TestWEBrick::RubyBin,
+      :DocumentRoot => File.dirname(__FILE__),
+      :CGIPathEnv => ENV['PATH'],
+    }
+    TestWEBrick.start_httpserver(config) do |server, addr, port|
+      http = Net::HTTP.new(addr, port)
+
+      req = Net::HTTP::Get.new("/webric~1.cgi/test")
+      http.request(req) do |res|
+        if windows?
+          assert_equal("200", res.code)
+          assert_equal("/test", res.body)
+        else
+          assert_equal("404", res.code)
+        end
+      end
+
+      req = Net::HTTP::Get.new("/.htaccess")
+      http.request(req) {|res| assert_equal("404", res.code) }
+      req = Net::HTTP::Get.new("/htacce~1")
+      http.request(req) {|res| assert_equal("404", res.code) }
+      req = Net::HTTP::Get.new("/HTACCE~1")
+      http.request(req) {|res| assert_equal("404", res.code) }
+    end
+  end
+
+  def test_script_disclosure
+    config = {
+      :CGIInterpreter => TestWEBrick::RubyBin,
+      :DocumentRoot => File.dirname(__FILE__),
+      :CGIPathEnv => ENV['PATH'],
+    }
+    TestWEBrick.start_httpserver(config) do |server, addr, port|
+      http = Net::HTTP.new(addr, port)
+
+      req = Net::HTTP::Get.new("/webrick.cgi/test")
+      http.request(req) do |res|
+        assert_equal("200", res.code)
+        assert_equal("/test", res.body)
+      end
+
+      response_assertion = Proc.new do |res|
+        if windows?
+          assert_equal("200", res.code)
+          assert_equal("/test", res.body)
+        else
+          assert_equal("404", res.code)
+        end
+      end
+      req = Net::HTTP::Get.new("/webrick.cgi%20/test")
+      http.request(req, &response_assertion)
+      req = Net::HTTP::Get.new("/webrick.cgi./test")
+      http.request(req, &response_assertion)
+      req = Net::HTTP::Get.new("/webrick.cgi::$DATA/test")
+      http.request(req, &response_assertion)
+    end
+  end
 end
diff --git a/test/webrick/utils.rb b/test/webrick/utils.rb
index f1e6e4b02..5db94e6cd 100644
--- a/test/webrick/utils.rb
+++ b/test/webrick/utils.rb
@@ -1,3 +1,10 @@
+begin
+  loadpath = $:.dup
+  $:.replace($: | [File.expand_path("../ruby", File.dirname(__FILE__))])
+  require 'envutil'
+ensure
+  $:.replace(loadpath)
+end
 require "webrick"
 begin
   require "webrick/https"
@@ -12,6 +19,11 @@ module TestWEBrick
     return self
   end
 
+  RubyBin = "\"#{EnvUtil.rubybin}\""
+  RubyBin << " \"-I#{File.expand_path("../..", File.dirname(__FILE__))}/lib\""
+  RubyBin << " \"-I#{File.dirname(EnvUtil.rubybin)}/.ext/common\""
+  RubyBin << " \"-I#{File.dirname(EnvUtil.rubybin)}/.ext/#{RUBY_PLATFORM}\""
+
   module_function
 
   def start_server(klass, config={}, &block)
diff --git a/test/webrick/webrick_long_filename.cgi b/test/webrick/webrick_long_filename.cgi
new file mode 100755
index 000000000..73ba72940
--- /dev/null
+++ b/test/webrick/webrick_long_filename.cgi
@@ -0,0 +1,36 @@
+#!ruby -d
+require "webrick/cgi"
+
+class TestApp < WEBrick::CGI
+  def do_GET(req, res)
+    res["content-type"] = "text/plain"
+    if (p = req.path_info) && p.length > 0
+      res.body = p
+    elsif (q = req.query).size > 0
+      res.body = q.keys.sort.collect{|key|
+        q[key].list.sort.collect{|v|
+          "#{key}=#{v}"
+        }.join(", ")
+      }.join(", ")
+    elsif %r{/$} =~ req.request_uri.to_s
+      res.body = ""
+      res.body << req.request_uri.to_s  << "\n"
+      res.body << req.script_name
+    elsif !req.cookies.empty?
+      res.body = req.cookies.inject(""){|result, cookie|
+        result << "%s=%s\n" % [cookie.name, cookie.value]
+      }
+      res.cookies << WEBrick::Cookie.new("Customer", "WILE_E_COYOTE")
+      res.cookies << WEBrick::Cookie.new("Shipping", "FedEx")
+    else
+      res.body = req.script_name
+    end
+  end
+
+  def do_POST(req, res)
+    do_GET(req, res)
+  end
+end
+
+cgi = TestApp.new
+cgi.start
diff --git a/version.h b/version.h
index a6d5133e2..24b2d6075 100644
--- a/version.h
+++ b/version.h
@@ -2,14 +2,14 @@
 #define RUBY_RELEASE_DATE "2008-05-18"
 #define RUBY_VERSION_CODE 186
 #define RUBY_RELEASE_CODE 20080518
-#define RUBY_PATCHLEVEL 118
+#define RUBY_PATCHLEVEL 119
 
 #define RUBY_VERSION_MAJOR 1
 #define RUBY_VERSION_MINOR 8
 #define RUBY_VERSION_TEENY 6
 #define RUBY_RELEASE_YEAR 2008
 #define RUBY_RELEASE_MONTH 5
-#define RUBY_RELEASE_DAY 18
+#define RUBY_RELEASE_DAY 21
 
 #ifdef RUBY_EXTERN
 RUBY_EXTERN const char ruby_version[];