From bd697a108a16199065363e58abeed3747a80e30f Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 5 Nov 2014 15:37:36 -0800 Subject: [PATCH] correctly escape backslashes in request path globs make sure that unreadable files are also not leaked CVE-2014-7829 --- .../lib/action_dispatch/middleware/static.rb | 4 +-- actionpack/test/dispatch/static_test.rb | 41 ++++++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/actionpack/lib/action_dispatch/middleware/static.rb b/actionpack/lib/action_dispatch/middleware/static.rb index df77021..2303447 100644 --- a/actionpack/lib/action_dispatch/middleware/static.rb +++ b/actionpack/lib/action_dispatch/middleware/static.rb @@ -19,7 +19,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) @@ -42,7 +42,7 @@ module ActionDispatch end def escape_glob_chars(path) - path.gsub(/[*?{}\[\]]/, "\\\\\\&") + path.gsub(/[*?{}\[\]\\]/, "\\\\\\&") end private diff --git a/actionpack/test/dispatch/static_test.rb b/actionpack/test/dispatch/static_test.rb index 4e9cdc3..1039413 100644 --- a/actionpack/test/dispatch/static_test.rb +++ b/actionpack/test/dispatch/static_test.rb @@ -1,5 +1,6 @@ # encoding: utf-8 require 'abstract_unit' +require 'fileutils' require 'rbconfig' module StaticTests @@ -164,6 +165,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)) -- 1.9.3 (Apple Git-50)