Skip to content

Commit

Permalink
Check for absolute paths in server URL before passing to find
Browse files Browse the repository at this point in the history
Various double slashes and URL encodings can bypass current checks.
In the case of the file existing, the server will 500 instead of 403
which leaks the existence but not the contents of the file.

Props to @eadz for finding this.
  • Loading branch information
josh committed Oct 28, 2014
1 parent fde5807 commit dbf5e0a
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 3 deletions.
2 changes: 1 addition & 1 deletion lib/sprockets/server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ def forbidden_request?(path)
#
# http://example.org/assets/../../../etc/passwd
#
path.include?("..")
path.include?("..") || Pathname.new(path).absolute?
end

# Returns a 403 Forbidden response tuple
Expand Down
16 changes: 14 additions & 2 deletions test/test_server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -198,10 +198,22 @@ def app
end

test "illegal require outside load path" do
get "/assets/../config/passwd"
get "/assets//etc/passwd"
assert_equal 403, last_response.status

get "/assets/%2e%2e/config/passwd"
get "/assets/%2fetc/passwd"
assert_equal 403, last_response.status

get "/assets//%2fetc/passwd"
assert_equal 403, last_response.status

get "/assets/%2f/etc/passwd"
assert_equal 403, last_response.status

get "/assets/../etc/passwd"
assert_equal 403, last_response.status

get "/assets/%2e%2e/etc/passwd"
assert_equal 403, last_response.status
end

Expand Down

0 comments on commit dbf5e0a

Please sign in to comment.