From 9c9da3b8a25174721a04e988f904b7dc3d7343d1 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Wed, 15 Oct 2014 11:59:48 -0700 Subject: [PATCH 1/3] 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. --- lib/sprockets/server.rb | 14 +++++++------- test/test_server.rb | 22 ++++++++++++++++++++-- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/lib/sprockets/server.rb b/lib/sprockets/server.rb index e9c2e59..e71f413 100644 --- a/lib/sprockets/server.rb +++ b/lib/sprockets/server.rb @@ -33,16 +33,16 @@ module Sprockets # 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 - # Strip fingerprint if fingerprint = path_fingerprint(path) path = path.sub("-#{fingerprint}", '') end + # URLs containing a `".."` are rejected for security reasons. + if forbidden_request?(path) + return forbidden_response + end + # Look up the asset. asset = find_asset(path, :bundle => !body_only?(env)) @@ -90,7 +90,7 @@ module Sprockets # # http://example.org/assets/../../../etc/passwd # - path.include?("..") + path.include?("..") || Pathname.new(path).absolute? end # Returns a 403 Forbidden response tuple @@ -222,7 +222,7 @@ module Sprockets # # => "0aa2105d29558f3eb790d411d7d8fb66" # def path_fingerprint(path) - path[/-([0-9a-f]{7,40})\.[^.]+$/, 1] + path[/-([0-9a-f]{7,40})\.[^.]+\z/, 1] end # URI.unescape is deprecated on 1.9. We need to use URI::Parser diff --git a/test/test_server.rb b/test/test_server.rb index 41e263d..c5f6a74 100644 --- a/test/test_server.rb +++ b/test/test_server.rb @@ -183,10 +183,28 @@ class TestServer < Sprockets::TestCase 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 + + get "/assets/.-0000000./etc/passwd" + assert_equal 403, last_response.status + + get "/assets/.-0000000./etc/passwd" assert_equal 403, last_response.status end