aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAaron Patterson <aaron.patterson@gmail.com>2014-10-10 16:00:03 -0700
committerAaron Patterson <aaron.patterson@gmail.com>2014-10-29 11:22:08 -0700
commit3437f260a5903b651bf8b8f751345fb288fa9052 (patch)
treece8f727e925959328b390de5b763176376b37b0e
parent53c845cb185036c71cc9793c4eb6bf4dc989307b (diff)
downloadrails-3437f260a5903b651bf8b8f751345fb288fa9052.tar.gz
rails-3437f260a5903b651bf8b8f751345fb288fa9052.tar.bz2
rails-3437f260a5903b651bf8b8f751345fb288fa9052.zip
FileHandler should not be called for files outside the root
FileHandler#matches? should return false for files that are outside the "root" path. Conflicts: actionpack/lib/action_dispatch/middleware/static.rb Conflicts: actionpack/lib/action_dispatch/middleware/static.rb actionpack/test/dispatch/static_test.rb
-rw-r--r--actionpack/lib/action_dispatch/middleware/static.rb21
-rw-r--r--actionpack/test/dispatch/static_test.rb18
2 files changed, 38 insertions, 1 deletions
diff --git a/actionpack/lib/action_dispatch/middleware/static.rb b/actionpack/lib/action_dispatch/middleware/static.rb
index a8d176560c..7f1117009d 100644
--- a/actionpack/lib/action_dispatch/middleware/static.rb
+++ b/actionpack/lib/action_dispatch/middleware/static.rb
@@ -12,7 +12,7 @@ module ActionDispatch
def match?(path)
path = path.dup
- full_path = path.empty? ? @root : File.join(@root, escape_glob_chars(unescape_path(path)))
+ full_path = path.empty? ? @root : File.join(@root, escape_glob_chars(clean_path_info(unescape_path(path))))
paths = "#{full_path}#{ext}"
matches = Dir[paths]
@@ -42,6 +42,25 @@ module ActionDispatch
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)
+ parts = path_info.split PATH_SEPS
+
+ clean = []
+
+ parts.each do |part|
+ next if part.empty? || part == '.'
+ part == '..' ? clean.pop : clean << part
+ end
+
+ clean.unshift '/' if parts.empty? || parts.first.empty?
+
+ ::File.join(*clean)
+ end
end
class Static
diff --git a/actionpack/test/dispatch/static_test.rb b/actionpack/test/dispatch/static_test.rb
index 856746c451..a035abdf08 100644
--- a/actionpack/test/dispatch/static_test.rb
+++ b/actionpack/test/dispatch/static_test.rb
@@ -140,10 +140,28 @@ class StaticTest < ActiveSupport::TestCase
[200, {"Content-Type" => "text/plain"}, ["Hello, World!"]]
}
App = ActionDispatch::Static.new(DummyApp, "#{FIXTURE_LOAD_PATH}/public", "public, max-age=60")
+ Root = "#{FIXTURE_LOAD_PATH}/public"
def setup
@app = App
+ @root = Root
end
include StaticTests
+
+ def test_custom_handler_called_when_file_is_outside_root
+ filename = 'shared.html.erb'
+ assert File.exist?(File.join(@root, '..', filename))
+ env = {
+ "REQUEST_METHOD"=>"GET",
+ "REQUEST_PATH"=>"/..%2F#{filename}",
+ "PATH_INFO"=>"/..%2F#{filename}",
+ "REQUEST_URI"=>"/..%2F#{filename}",
+ "HTTP_VERSION"=>"HTTP/1.1",
+ "SERVER_NAME"=>"localhost",
+ "SERVER_PORT"=>"8080",
+ "QUERY_STRING"=>""
+ }
+ assert_equal(DummyApp.call(nil), @app.call(env))
+ end
end