Permalink
Browse files

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
  • Loading branch information...
1 parent dc4e578 commit 4d832126380a18a3145c47dfbc57021f96e9a89e @jfirebaugh jfirebaugh committed with josh Feb 9, 2012
Showing with 10 additions and 7 deletions.
  1. +7 −7 lib/sprockets/server.rb
  2. +3 −0 test/test_server.rb
View
@@ -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
View
@@ -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

0 comments on commit 4d83212

Please sign in to comment.