Skip to content
Browse files

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

Conflicts:
	actionpack/lib/action_dispatch/middleware/static.rb
  • Loading branch information...
1 parent 9c37d8e commit 4dacedf983257aef38a8ebedb2d9a9c8fead8238 @tenderlove tenderlove committed Nov 5, 2014
Showing with 45 additions and 2 deletions.
  1. +4 −2 actionpack/lib/action_dispatch/middleware/static.rb
  2. +41 −0 actionpack/test/dispatch/static_test.rb
View
6 actionpack/lib/action_dispatch/middleware/static.rb
@@ -15,7 +15,7 @@ def match?(path)
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)
@@ -38,14 +38,16 @@ def unescape_path(path)
end
def escape_glob_chars(path)
- path.gsub(/[*?{}\[\]]/, "\\\\\\&")
+ path.force_encoding('binary') if path.respond_to? :force_encoding
+ path.gsub(/[*?{}\[\]\\]/, "\\\\\\&")
end
private
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 = []
View
41 actionpack/test/dispatch/static_test.rb
@@ -1,4 +1,5 @@
require 'abstract_unit'
+require 'fileutils'
require 'rbconfig'
module StaticTests
@@ -145,6 +146,46 @@ def setup
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))

0 comments on commit 4dacedf

Please sign in to comment.
Something went wrong with that request. Please try again.