summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorgotoyuzo <gotoyuzo@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>2008-05-20 16:35:25 +0000
committergotoyuzo <gotoyuzo@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>2008-05-20 16:35:25 +0000
commitbc9e9376332bdba8601a71ff18eada31cb90de6b (patch)
tree9ea7dd0ebd4baa557a202b8c68438543cc1117d7
parent10a21478845b788b01bb3b6913d55c3d565efd40 (diff)
* 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
-rw-r--r--ChangeLog16
-rw-r--r--lib/webrick/httpservlet/abstract.rb2
-rw-r--r--lib/webrick/httpservlet/cgi_runner.rb4
-rw-r--r--lib/webrick/httpservlet/filehandler.rb60
-rw-r--r--test/webrick/.htaccess1
-rw-r--r--test/webrick/test_cgi.rb9
-rw-r--r--test/webrick/test_filehandler.rb82
-rw-r--r--test/webrick/utils.rb12
-rwxr-xr-xtest/webrick/webrick_long_filename.cgi36
-rw-r--r--version.h4
10 files changed, 191 insertions, 35 deletions
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 <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 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 << "<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 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[];