From c47a4ae57b958c8e5b79d2b4636409f54d046d96 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 10 Oct 2014 16:00:03 -0700 Subject: [PATCH] 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 --- .../lib/action_dispatch/middleware/static.rb | 22 +++++++++++++++++++++- actionpack/test/dispatch/static_test.rb | 22 ++++++++++++++++++++-- 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/actionpack/lib/action_dispatch/middleware/static.rb b/actionpack/lib/action_dispatch/middleware/static.rb index 2764584..df77021 100644 --- a/actionpack/lib/action_dispatch/middleware/static.rb +++ b/actionpack/lib/action_dispatch/middleware/static.rb @@ -14,7 +14,8 @@ module ActionDispatch path = unescape_path(path) return false unless path.valid_encoding? - full_path = path.empty? ? @root : File.join(@root, escape_glob_chars(path)) + full_path = path.empty? ? @root : File.join(@root, + clean_path_info(escape_glob_chars(path))) paths = "#{full_path}#{ext}" matches = Dir[paths] @@ -43,6 +44,25 @@ module ActionDispatch def escape_glob_chars(path) 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 d3fe9df..79a7975 100644 --- a/actionpack/test/dispatch/static_test.rb +++ b/actionpack/test/dispatch/static_test.rb @@ -147,7 +147,8 @@ class StaticTest < ActiveSupport::TestCase } def setup - @app = ActionDispatch::Static.new(DummyApp, "#{FIXTURE_LOAD_PATH}/public", "public, max-age=60") + @root = "#{FIXTURE_LOAD_PATH}/public" + @app = ActionDispatch::Static.new(DummyApp, @root, "public, max-age=60") end def public_path @@ -155,11 +156,28 @@ class StaticTest < ActiveSupport::TestCase 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 class StaticEncodingTest < StaticTest def setup - @app = ActionDispatch::Static.new(DummyApp, "#{FIXTURE_LOAD_PATH}/公共", "public, max-age=60") + @root = "#{FIXTURE_LOAD_PATH}/公共" + @app = ActionDispatch::Static.new(DummyApp, @root, "public, max-age=60") end def public_path -- 1.9.3 (Apple Git-50)