aboutsummaryrefslogtreecommitdiffstats
path: root/actionpack
diff options
context:
space:
mode:
authorJohn Hawthorn <john@hawthorn.email>2019-04-01 16:35:07 -0700
committerJohn Hawthorn <john@hawthorn.email>2019-04-03 09:02:28 -0700
commiteb52904eb5c19ab4e8ff7a7d4f501fe5e1142ad0 (patch)
tree69e7486d27a6c0555473cea655cc4ab36c9f6c1a /actionpack
parentbeb0bc9907a31d0cbd2ca68c79c57a9e375761e8 (diff)
downloadrails-eb52904eb5c19ab4e8ff7a7d4f501fe5e1142ad0.tar.gz
rails-eb52904eb5c19ab4e8ff7a7d4f501fe5e1142ad0.tar.bz2
rails-eb52904eb5c19ab4e8ff7a7d4f501fe5e1142ad0.zip
Always reject files external to app
Previously, when using `render file:`, it was possible to render files not only at an absolute path or relative to the current directory, but relative to ANY view paths. This was probably done for absolutely maximum compatibility when addressing CVE-2016-0752, but I think is unlikely to be used in practice. Tihs commit removes the ability to `render file:` with a path relative to a non-fallback view path. Make FallbackResolver.new private To ensure nobody is making FallbackResolvers other than "/" and "". Make reject_files_external_... no-op for fallbacks Because there are only two values used for path: "" and "/", and File.join("", "") == File.join("/", "") == "/", this method was only testing that the absolute paths started at "/" (which of course all do). This commit doesn't change any behaviour, but it makes it explicit that the FallbackFileSystemResolver works this way. Remove outside_app_allowed argument Deprecate find_all_anywhere This is now equivalent to find_all Remove outside_app argument Deprecate find_file for find Both LookupContext#find_file and PathSet#find_file are now equivalent to their respective #find methods.
Diffstat (limited to 'actionpack')
-rw-r--r--actionpack/test/controller/new_base/render_file_test.rb4
-rw-r--r--actionpack/test/controller/render_test.rb17
2 files changed, 12 insertions, 9 deletions
diff --git a/actionpack/test/controller/new_base/render_file_test.rb b/actionpack/test/controller/new_base/render_file_test.rb
index 82325c5bb2..01d0223519 100644
--- a/actionpack/test/controller/new_base/render_file_test.rb
+++ b/actionpack/test/controller/new_base/render_file_test.rb
@@ -17,12 +17,12 @@ module RenderFile
def relative_path
@secret = "in the sauce"
- render file: "../../fixtures/test/render_file_with_ivar"
+ render file: "../actionpack/test/fixtures/test/render_file_with_ivar"
end
def relative_path_with_dot
@secret = "in the sauce"
- render file: "../../fixtures/test/dot.directory/render_file_with_ivar"
+ render file: "../actionpack/test/fixtures/test/dot.directory/render_file_with_ivar"
end
def pathname
diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb
index 6d198ca42f..8bb6617eaa 100644
--- a/actionpack/test/controller/render_test.rb
+++ b/actionpack/test/controller/render_test.rb
@@ -323,11 +323,12 @@ class ExpiresInRenderTest < ActionController::TestCase
end
def test_dynamic_render_with_file
- # This is extremely bad, but should be possible to do.
assert File.exist?(File.expand_path("../../test/abstract_unit.rb", __dir__))
- response = assert_deprecated { get :dynamic_render_with_file, params: { id: '../\\../test/abstract_unit.rb' } }
- assert_equal File.read(File.expand_path("../../test/abstract_unit.rb", __dir__)),
- response.body
+ assert_deprecated do
+ assert_raises ActionView::MissingTemplate do
+ get :dynamic_render_with_file, params: { id: '../\\../test/abstract_unit.rb' }
+ end
+ end
end
def test_dynamic_render_with_absolute_path
@@ -351,9 +352,11 @@ class ExpiresInRenderTest < ActionController::TestCase
def test_permitted_dynamic_render_file_hash
assert File.exist?(File.expand_path("../../test/abstract_unit.rb", __dir__))
- response = assert_deprecated { get :dynamic_render_permit, params: { id: { file: '../\\../test/abstract_unit.rb' } } }
- assert_equal File.read(File.expand_path("../../test/abstract_unit.rb", __dir__)),
- response.body
+ assert_deprecated do
+ assert_raises ActionView::MissingTemplate do
+ get :dynamic_render_permit, params: { id: { file: '../\\../test/abstract_unit.rb' } }
+ end
+ end
end
def test_dynamic_render_file_hash