From 4d832126380a18a3145c47dfbc57021f96e9a89e Mon Sep 17 00:00:00 2001
From: John Firebaugh <john.firebaugh@gmail.com>
Date: Wed, 8 Feb 2012 19:17:12 -0800
Subject: [PATCH] Check for directory traversal after unescaping

The `forbidden_request?` check could be trivially bypassed
by percent encoding .. as %2e%2e.

After auditing Sprockets and Hike and fuzzing a simple
server, I don't believe this is exploitable. However,
better safe than sorry/defense in depth/etc.

Conflicts:

        lib/sprockets/server.rb
---
 lib/sprockets/server.rb | 14 +++++++-------
 test/test_server.rb     |  3 +++
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/lib/sprockets/server.rb b/lib/sprockets/server.rb
index 0b5f88fd6..f69133831 100644
--- a/lib/sprockets/server.rb
+++ b/lib/sprockets/server.rb
@@ -25,11 +25,6 @@ def call(env)
 
       msg = "Served asset #{env['PATH_INFO']} -"
 
-      # URLs containing a `".."` are rejected for security reasons.
-      if forbidden_request?(env)
-        return forbidden_response
-      end
-
       # Mark session as "skipped" so no `Set-Cookie` header is set
       env['rack.session.options'] ||= {}
       env['rack.session.options'][:defer] = true
@@ -38,6 +33,11 @@ def call(env)
       # Extract the path from everything after the leading slash
       path = unescape(env['PATH_INFO'].to_s.sub(/^\//, ''))
 
+      # URLs containing a `".."` are rejected for security reasons.
+      if forbidden_request?(path)
+        return forbidden_response
+      end
+
       # Look up the asset.
       asset = find_asset(path)
       asset.to_a if asset
@@ -82,12 +82,12 @@ def call(env)
     end
 
     private
-      def forbidden_request?(env)
+      def forbidden_request?(path)
         # Prevent access to files elsewhere on the file system
         #
         #     http://example.org/assets/../../../etc/passwd
         #
-        env["PATH_INFO"].include?("..")
+        path.include?("..")
       end
 
       # Returns a 403 Forbidden response tuple
diff --git a/test/test_server.rb b/test/test_server.rb
index 0e2ff29a1..fb7d4415f 100644
--- a/test/test_server.rb
+++ b/test/test_server.rb
@@ -200,6 +200,9 @@ def app
   test "illegal require outside load path" do
     get "/assets/../config/passwd"
     assert_equal 403, last_response.status
+
+    get "/assets/%2e%2e/config/passwd"
+    assert_equal 403, last_response.status
   end
 
   test "add new source to tree" do