Merge pull request #1065 from jkowens/fix-null-byte

Return 400 if Rack::File or Rack::Directory path contains null byte

lib/rack/directory.rb

@@ -71,14 +71,26 @@ def get(env)
       script_name = env[SCRIPT_NAME]
       path_info = Utils.unescape_path(env[PATH_INFO])
 
-      if forbidden = check_forbidden(path_info)
+      if bad_request = check_bad_request(path_info)
+        bad_request
+      elsif forbidden = check_forbidden(path_info)
         forbidden
       else
         path = ::File.join(@root, path_info)
         list_path(env, path, path_info, script_name)
       end
     end
 
+    def check_bad_request(path_info)
+      return if Utils.valid_path?(path_info)
+
+      body = "Bad Request\n"
+      size = body.bytesize
+      return [400, {CONTENT_TYPE => "text/plain",
+        CONTENT_LENGTH => size.to_s,
+        "X-Cascade" => "pass"}, [body]]
+    end
+
     def check_forbidden(path_info)
       return unless path_info.include? ".."
 

lib/rack/file.rb

@@ -38,8 +38,9 @@ def get(env)
       end
 
       path_info = Utils.unescape_path request.path_info
-      clean_path_info = Utils.clean_path_info(path_info)
+      return fail(400, "Bad Request") unless Utils.valid_path?(path_info)
 
+      clean_path_info = Utils.clean_path_info(path_info)
       path = ::File.join(@root, clean_path_info)
 
       available = begin

lib/rack/utils.rb

@@ -609,5 +609,12 @@ def clean_path_info(path_info)
     end
     module_function :clean_path_info
 
+    NULL_BYTE = "\0".freeze
+
+    def valid_path?(path)
+      path.valid_encoding? && !path.include?(NULL_BYTE)
+    end
+    module_function :valid_path?
+
   end
 end

test/spec_directory.rb

@@ -63,6 +63,13 @@ def setup
     assert_match(res, /passed!/)
   end
 
+  it "serve uri with URL encoded null byte (%00) in filenames" do
+    res = Rack::MockRequest.new(Rack::Lint.new(app))
+      .get("/cgi/test%00")
+
+    res.must_be :bad_request?
+  end
+
   it "not allow directory traversal" do
     res = Rack::MockRequest.new(Rack::Lint.new(app)).
       get("/cgi/../test")

test/spec_file.rb

@@ -68,6 +68,11 @@ def file(*args)
     assert_match(res, /ruby/)
   end
 
+  it "serve uri with URL encoded null byte (%00) in filenames" do
+    res = Rack::MockRequest.new(file(DOCROOT)).get("/cgi/test%00")
+    res.must_be :bad_request?
+  end
+
   it "allow safe directory traversal" do
     req = Rack::MockRequest.new(file(DOCROOT))