aboutsummaryrefslogtreecommitdiffstats
path: root/actionpack
diff options
context:
space:
mode:
authorAaron Patterson <aaron.patterson@gmail.com>2014-11-05 15:37:36 -0800
committerAaron Patterson <aaron.patterson@gmail.com>2014-11-16 11:23:44 -0800
commit307402febd448646df796e7dabbbaa5734f641aa (patch)
tree828d20f68284938721aa0b5447b6e5626fabb526 /actionpack
parent03366b14d15aeebf255a54b2878dcc6206c0c372 (diff)
downloadrails-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.rb5
-rw-r--r--actionpack/test/dispatch/static_test.rb41
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))