Failing test for pass traversal issue.

git-svn-id: svn+ssh://rubyforge.org/var/svn/mongrel/branches/stable_1-1@963 19e92222-5c0b-0410-8929-a290d50e31e9

2 files changed, 29 insertions, 13 deletions

diff --git a/lib/mongrel/handlers.rb b/lib/mongrel/handlers.rb
index 9b9798e..2a6bca4 100644
--- a/lib/mongrel/handlers.rb
+++ b/lib/mongrel/handlers.rb
@@ -8,7 +8,6 @@ require 'mongrel/stats'
 require 'zlib'
 require 'yaml'
 
-
 module Mongrel
 
   # You implement your application handler with this.  It's very light giving
@@ -102,7 +101,8 @@ module Mongrel
   #
   # If you pass nil as the root path, it will not check any locations or
   # expand any paths. This lets you serve files from multiple drives
-  # on win32.
+  # on win32. It should probably not be used in a public-facing way
+  # without additional checks.
   #
   # The default content type is "text/plain; charset=ISO-8859-1" but you
   # can change it anything you want using the DirHandler.default_content_type
@@ -120,7 +120,7 @@ module Mongrel
     # You give it the path to the directory root and and optional listing_allowed and index_html
     def initialize(path, listing_allowed=true, index_html="index.html")
       @path = File.expand_path(path) if path
-      @listing_allowed=listing_allowed
+      @listing_allowed = listing_allowed
       @index_html = index_html
       @default_content_type = "application/octet-stream".freeze
     end
@@ -132,12 +132,8 @@ module Mongrel
       # Add the drive letter or root path
       req_path = File.join(@path, req_path) if @path
       req_path = File.expand_path req_path
-     
-      # do not remove the check for @path at the beginning, it's what prevents
-      # the serving of arbitrary files (and good programmer Rule #1 Says: If
-      # you don't understand something, it's not because I'm stupid, it's
-      # because you are).
-      if req_path.index(@path) == 0 and File.exist? req_path
+      
+      if File.exist? req_path # and (!@path or req_path.index(@path) == 0)
         # It exists and it's in the right location
         if File.directory? req_path
           # The request is for a directory
@@ -157,7 +153,7 @@ module Mongrel
           return req_path
         end
       else
-        # does not exist or isn't in the right spot or isn't valid because not start with @path
+        # does not exist or isn't in the right spot
         return nil
       end
     end
diff --git a/test/test_handlers.rb b/test/test_handlers.rb
index 72abbbc..1005dd0 100644
--- a/test/test_handlers.rb
+++ b/test/test_handlers.rb
@@ -49,11 +49,17 @@ class HandlersTest < Test::Unit::TestCase
         uri "/relative", :handler => Mongrel::DirHandler.new(nil, listing_allowed=false, index_html="none")
       end
     end
+    
+    File.open("/tmp/testfile", 'w') do
+      # Do nothing
+    end
+    
     @config.run
   end
 
   def teardown
     @config.stop(false, true)
+    File.delete "/tmp/testfile"
   end
 
   def test_more_web_server
@@ -66,14 +72,28 @@ class HandlersTest < Test::Unit::TestCase
           "http://localhost:9998/files_nodir/rdoc/",
           "http://localhost:9998/status",
     ])
-
-    # XXX This can't possibly have good coverage.
     check_status res, String
   end
+  
+  def test_nil_dirhandler
+    # Camping uses this internally
+    handler = Mongrel::DirHandler.new(nil, false)  
+    assert handler.can_serve("/tmp/testfile")
+    # Not a bug! A nil @file parameter is the only circumstance under which
+    # we are allowed to serve any existing file
+    assert handler.can_serve("../../../../../../../../../../tmp/testfile")
+  end
+  
+  def test_non_nil_dirhandler_is_not_vulnerable_to_path_traversal
+    # The famous security bug of Mongrel 1.1.2
+    handler = Mongrel::DirHandler.new("/doc", false)
+    assert_nil handler.can_serve("/tmp/testfile")
+    assert_nil handler.can_serve("../../../../../../../../../../tmp/testfile")
+  end
 
   def test_deflate
     Net::HTTP.start("localhost", 9998) do |h|
-      # test that no accept-encoding returns a non-deflated response
+      # Test that no accept-encoding returns a non-deflated response
       req = h.get("/dumb")
       assert(
         !req['Content-Encoding'] ||