diff options
author | Aaron Patterson <aaron.patterson@gmail.com> | 2014-11-05 15:37:36 -0800 |
---|---|---|
committer | Aaron Patterson <aaron.patterson@gmail.com> | 2014-11-16 11:23:44 -0800 |
commit | 307402febd448646df796e7dabbbaa5734f641aa (patch) | |
tree | 828d20f68284938721aa0b5447b6e5626fabb526 /actionpack | |
parent | 03366b14d15aeebf255a54b2878dcc6206c0c372 (diff) | |
download | rails-307402febd448646df796e7dabbbaa5734f641aa.tar.gz rails-307402febd448646df796e7dabbbaa5734f641aa.tar.bz2 rails-307402febd448646df796e7dabbbaa5734f641aa.zip |
correctly escape backslashes in request path globs
Conflicts:
actionpack/lib/action_dispatch/middleware/static.rb
make sure that unreadable files are also not leaked
CVE-2014-7829
Diffstat (limited to 'actionpack')
-rw-r--r-- | actionpack/lib/action_dispatch/middleware/static.rb | 5 | ||||
-rw-r--r-- | actionpack/test/dispatch/static_test.rb | 41 |
2 files changed, 44 insertions, 2 deletions
diff --git a/actionpack/lib/action_dispatch/middleware/static.rb b/actionpack/lib/action_dispatch/middleware/static.rb index 7f1117009d..70632f2aea 100644 --- a/actionpack/lib/action_dispatch/middleware/static.rb +++ b/actionpack/lib/action_dispatch/middleware/static.rb @@ -16,7 +16,7 @@ module ActionDispatch paths = "#{full_path}#{ext}" matches = Dir[paths] - match = matches.detect { |m| File.file?(m) } + match = matches.detect { |m| File.file?(m) && File.readable?(m) } if match match.sub!(@compiled_root, '') ::Rack::Utils.escape(match) @@ -40,7 +40,7 @@ module ActionDispatch def escape_glob_chars(path) path.force_encoding('binary') if path.respond_to? :force_encoding - path.gsub(/[*?{}\[\]]/, "\\\\\\&") + path.gsub(/[*?{}\[\]\\]/, "\\\\\\&") end private @@ -48,6 +48,7 @@ module ActionDispatch PATH_SEPS = Regexp.union(*[::File::SEPARATOR, ::File::ALT_SEPARATOR].compact) def clean_path_info(path_info) + path_info.force_encoding('binary') if path_info.respond_to? :force_encoding parts = path_info.split PATH_SEPS clean = [] diff --git a/actionpack/test/dispatch/static_test.rb b/actionpack/test/dispatch/static_test.rb index a035abdf08..e4a71bcba2 100644 --- a/actionpack/test/dispatch/static_test.rb +++ b/actionpack/test/dispatch/static_test.rb @@ -1,4 +1,5 @@ require 'abstract_unit' +require 'fileutils' require 'rbconfig' module StaticTests @@ -149,6 +150,46 @@ class StaticTest < ActiveSupport::TestCase include StaticTests + def test_custom_handler_called_when_file_is_not_readable + filename = 'unreadable.html.erb' + target = File.join(@root, filename) + FileUtils.touch target + File.chmod 0200, target + assert File.exist? target + assert !File.readable?(target) + path = "/#{filename}" + env = { + "REQUEST_METHOD"=>"GET", + "REQUEST_PATH"=> path, + "PATH_INFO"=> path, + "REQUEST_URI"=> path, + "HTTP_VERSION"=>"HTTP/1.1", + "SERVER_NAME"=>"localhost", + "SERVER_PORT"=>"8080", + "QUERY_STRING"=>"" + } + assert_equal(DummyApp.call(nil), @app.call(env)) + ensure + File.unlink target + end + + def test_custom_handler_called_when_file_is_outside_root_backslash + filename = 'shared.html.erb' + assert File.exist?(File.join(@root, '..', filename)) + path = "/%5C..%2F#{filename}" + env = { + "REQUEST_METHOD"=>"GET", + "REQUEST_PATH"=> path, + "PATH_INFO"=> path, + "REQUEST_URI"=> path, + "HTTP_VERSION"=>"HTTP/1.1", + "SERVER_NAME"=>"localhost", + "SERVER_PORT"=>"8080", + "QUERY_STRING"=>"" + } + assert_equal(DummyApp.call(nil), @app.call(env)) + end + def test_custom_handler_called_when_file_is_outside_root filename = 'shared.html.erb' assert File.exist?(File.join(@root, '..', filename)) |