Skip to content

Commit 3043d14

Browse files
committed
Check for absolute paths in server URL before passing to find
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.
1 parent abd1827 commit 3043d14

File tree

2 files changed

+27
-9
lines changed

2 files changed

+27
-9
lines changed

lib/sprockets/server.rb

+7-7
Original file line numberDiff line numberDiff line change
@@ -33,16 +33,16 @@ def call(env)
3333
# Extract the path from everything after the leading slash
3434
path = unescape(env['PATH_INFO'].to_s.sub(/^\//, ''))
3535

36-
# URLs containing a `".."` are rejected for security reasons.
37-
if forbidden_request?(path)
38-
return forbidden_response
39-
end
40-
4136
# Strip fingerprint
4237
if fingerprint = path_fingerprint(path)
4338
path = path.sub("-#{fingerprint}", '')
4439
end
4540

41+
# URLs containing a `".."` are rejected for security reasons.
42+
if forbidden_request?(path)
43+
return forbidden_response
44+
end
45+
4646
# Look up the asset.
4747
asset = find_asset(path, :bundle => !body_only?(env))
4848

@@ -90,7 +90,7 @@ def forbidden_request?(path)
9090
#
9191
# http://example.org/assets/../../../etc/passwd
9292
#
93-
path.include?("..")
93+
path.include?("..") || Pathname.new(path).absolute?
9494
end
9595

9696
# Returns a 403 Forbidden response tuple
@@ -222,7 +222,7 @@ def headers(env, asset, length)
222222
# # => "0aa2105d29558f3eb790d411d7d8fb66"
223223
#
224224
def path_fingerprint(path)
225-
path[/-([0-9a-f]{7,40})\.[^.]+$/, 1]
225+
path[/-([0-9a-f]{7,40})\.[^.]+\z/, 1]
226226
end
227227

228228
# URI.unescape is deprecated on 1.9. We need to use URI::Parser

test/test_server.rb

+20-2
Original file line numberDiff line numberDiff line change
@@ -182,10 +182,28 @@ def app
182182
end
183183

184184
test "illegal require outside load path" do
185-
get "/assets/../config/passwd"
185+
get "/assets//etc/passwd"
186186
assert_equal 403, last_response.status
187187

188-
get "/assets/%2e%2e/config/passwd"
188+
get "/assets/%2fetc/passwd"
189+
assert_equal 403, last_response.status
190+
191+
get "/assets//%2fetc/passwd"
192+
assert_equal 403, last_response.status
193+
194+
get "/assets/%2f/etc/passwd"
195+
assert_equal 403, last_response.status
196+
197+
get "/assets/../etc/passwd"
198+
assert_equal 403, last_response.status
199+
200+
get "/assets/%2e%2e/etc/passwd"
201+
assert_equal 403, last_response.status
202+
203+
get "/assets/.-0000000./etc/passwd"
204+
assert_equal 403, last_response.status
205+
206+
get "/assets/.-0000000./etc/passwd"
189207
assert_equal 403, last_response.status
190208
end
191209

0 commit comments

Comments
 (0)