From 4a6e0bc42c3e0dab34f02253089096d9e9004cd0 Mon Sep 17 00:00:00 2001
From: Jordan Owens <jkowens@gmail.com>
Date: Tue, 1 Dec 2015 20:18:15 -0500
Subject: Return 400 status if request for static file includes null byte

File paths cannot contain null byte characters and methods that do path
operations such as Rack::Utils#clean_path_info will raise unwanted
errors.
---
 lib/rack/directory.rb  | 14 +++++++++++++-
 lib/rack/file.rb       |  3 ++-
 lib/rack/utils.rb      |  7 +++++++
 test/spec_directory.rb |  7 +++++++
 test/spec_file.rb      |  5 +++++
 5 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/lib/rack/directory.rb b/lib/rack/directory.rb
index 5baee3c8..e642975c 100644
--- a/lib/rack/directory.rb
+++ b/lib/rack/directory.rb
@@ -65,7 +65,9 @@ table { width:100%%; }
       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)
@@ -73,6 +75,16 @@ table { width:100%%; }
       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? ".."
 
diff --git a/lib/rack/file.rb b/lib/rack/file.rb
index 227a7284..0a257b3d 100644
--- a/lib/rack/file.rb
+++ b/lib/rack/file.rb
@@ -38,8 +38,9 @@ module Rack
       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
diff --git a/lib/rack/utils.rb b/lib/rack/utils.rb
index 74dc1de5..7b842125 100644
--- a/lib/rack/utils.rb
+++ b/lib/rack/utils.rb
@@ -609,5 +609,12 @@ module Rack
     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
diff --git a/test/spec_directory.rb b/test/spec_directory.rb
index 1a97e9e5..3801b9d7 100644
--- a/test/spec_directory.rb
+++ b/test/spec_directory.rb
@@ -63,6 +63,13 @@ describe Rack::Directory do
     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")
diff --git a/test/spec_file.rb b/test/spec_file.rb
index 353dcdfe..3106e629 100644
--- a/test/spec_file.rb
+++ b/test/spec_file.rb
@@ -68,6 +68,11 @@ describe Rack::File do
     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))
 
-- 
cgit v1.2.3-24-ge0c7