From bc9e9376332bdba8601a71ff18eada31cb90de6b Mon Sep 17 00:00:00 2001 From: gotoyuzo Date: Tue, 20 May 2008 16:35:25 +0000 Subject: * 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: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_1_8_5@16495 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- ChangeLog | 16 +++++++ lib/webrick/httpservlet/abstract.rb | 2 +- lib/webrick/httpservlet/cgi_runner.rb | 4 +- lib/webrick/httpservlet/filehandler.rb | 60 ++++++++++++++++--------- test/webrick/.htaccess | 1 + test/webrick/test_cgi.rb | 9 +--- test/webrick/test_filehandler.rb | 82 ++++++++++++++++++++++++++++++++-- test/webrick/utils.rb | 12 +++++ test/webrick/webrick_long_filename.cgi | 36 +++++++++++++++ version.h | 4 +- 10 files changed, 191 insertions(+), 35 deletions(-) create mode 100644 test/webrick/.htaccess create mode 100755 test/webrick/webrick_long_filename.cgi diff --git a/ChangeLog b/ChangeLog index f010082011..287909dd3f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,19 @@ +Wed May 21 01:32:56 2008 GOTOU Yuuzou + + * 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 * 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 03861e8fc7..5375c4622d 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 1069a68d58..006abd458e 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 c8278b7b85..24f59d7142 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 << "Size\n" res.body << "
\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 0000000000..69d4659b9f --- /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 7e3f4ee5f0..b39fe2661c 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 e1299a4589..2c7d97fa70 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 f1e6e4b027..5db94e6cd5 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 0000000000..73ba729407 --- /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 cbbf9e5624..345183737e 100644 --- a/version.h +++ b/version.h @@ -2,14 +2,14 @@ #define RUBY_RELEASE_DATE "2008-05-18" #define RUBY_VERSION_CODE 185 #define RUBY_RELEASE_CODE 20080518 -#define RUBY_PATCHLEVEL 119 +#define RUBY_PATCHLEVEL 120 #define RUBY_VERSION_MAJOR 1 #define RUBY_VERSION_MINOR 8 #define RUBY_VERSION_TEENY 5 #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[]; -- cgit v1.2.3