diff options
author | Aaron Patterson <aaron.patterson@gmail.com> | 2016-01-20 10:39:19 -0800 |
---|---|---|
committer | Aaron Patterson <aaron.patterson@gmail.com> | 2016-01-22 15:01:49 -0800 |
commit | 18269d250fa58001ce7d8318571546aa90412975 (patch) | |
tree | bb44a4b20c0964b201d38ed864f7ad6b19b3fb60 /actionpack | |
parent | cdabc95608336dbea7b6a3a3e925de5bbd5313ba (diff) | |
download | rails-18269d250fa58001ce7d8318571546aa90412975.tar.gz rails-18269d250fa58001ce7d8318571546aa90412975.tar.bz2 rails-18269d250fa58001ce7d8318571546aa90412975.zip |
allow :file to be outside rails root, but anything else must be inside the rails view directory
Conflicts:
actionpack/test/controller/render_test.rb
actionview/lib/action_view/template/resolver.rb
CVE-2016-0752
Diffstat (limited to 'actionpack')
-rw-r--r-- | actionpack/lib/action_view/template/resolver.rb | 17 | ||||
-rw-r--r-- | actionpack/test/controller/new_base/render_file_test.rb | 18 | ||||
-rw-r--r-- | actionpack/test/controller/render_test.rb | 31 | ||||
-rw-r--r-- | actionpack/test/template/render_test.rb | 7 |
4 files changed, 69 insertions, 4 deletions
diff --git a/actionpack/lib/action_view/template/resolver.rb b/actionpack/lib/action_view/template/resolver.rb index 47ea8a3c9b..c6db6685e4 100644 --- a/actionpack/lib/action_view/template/resolver.rb +++ b/actionpack/lib/action_view/template/resolver.rb @@ -110,6 +110,9 @@ module ActionView super() end + cattr_accessor :allow_external_files, instance_reader: false, instance_writer: false + self.allow_external_files = false + private def find_templates(name, prefix, partial, details) @@ -122,6 +125,10 @@ module ActionView template_paths = find_template_paths query + unless self.class.allow_external_files + template_paths = reject_files_external_to_app(template_paths) + end + template_paths.map { |template| handler, format = extract_handler_and_format(template, formats) contents = File.binread template @@ -133,6 +140,10 @@ module ActionView } end + def reject_files_external_to_app(files) + files.reject { |filename| !inside_path?(@path, filename) } + end + if RUBY_VERSION >= '2.2.0' def find_template_paths(query) Dir[query].reject { |filename| @@ -153,6 +164,12 @@ module ActionView end end + def inside_path?(path, filename) + filename = File.expand_path(filename) + path = File.join(path, '') + filename.start_with?(path) + end + # Helper for building query glob string based on resolver's pattern. def build_query(path, details) query = @pattern.dup diff --git a/actionpack/test/controller/new_base/render_file_test.rb b/actionpack/test/controller/new_base/render_file_test.rb index a961cbf849..c0e23db457 100644 --- a/actionpack/test/controller/new_base/render_file_test.rb +++ b/actionpack/test/controller/new_base/render_file_test.rb @@ -72,13 +72,23 @@ module RenderFile end test "rendering a relative path" do - get :relative_path - assert_response "The secret is in the sauce\n" + begin + ActionView::PathResolver.allow_external_files = true + get :relative_path + assert_response "The secret is in the sauce\n" + ensure + ActionView::PathResolver.allow_external_files = false + end end test "rendering a relative path with dot" do - get :relative_path_with_dot - assert_response "The secret is in the sauce\n" + begin + ActionView::PathResolver.allow_external_files = true + get :relative_path_with_dot + assert_response "The secret is in the sauce\n" + ensure + ActionView::PathResolver.allow_external_files = false + end end test "rendering a Pathname" do diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 3964540def..6210524cef 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -60,6 +60,16 @@ class TestController < ActionController::Base end end + def dynamic_render + render params[:id] # => String, Hash + end + + def dynamic_render_with_file + # This is extremely bad, but should be possible to do. + file = params[:id] # => String, Hash + render file: file + end + def conditional_hello_with_public_header if stale?(:last_modified => Time.now.utc.beginning_of_day, :etag => [:foo, 123], :public => true) render :action => 'hello_world' @@ -235,6 +245,27 @@ class TestController < ActionController::Base render :inline => "<%= action_name %>" end + def test_dynamic_render_with_file + # This is extremely bad, but should be possible to do. + assert File.exist?(File.join(File.dirname(__FILE__), '../../test/abstract_unit.rb')) + response = get :dynamic_render_with_file, { id: '../\\../test/abstract_unit.rb' } + assert_equal File.read(File.join(File.dirname(__FILE__), '../../test/abstract_unit.rb')), + response.body + end + + def test_dynamic_render + assert File.exist?(File.join(File.dirname(__FILE__), '../../test/abstract_unit.rb')) + assert_raises ActionView::MissingTemplate do + get :dynamic_render, { id: '../\\../test/abstract_unit.rb' } + end + end + + def test_dynamic_render_file_hash + assert_raises ArgumentError do + get :dynamic_render, { id: { file: '../\\../test/abstract_unit.rb' } } + end + end + def accessing_controller_name_in_template render :inline => "<%= controller_name %>" end diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb index 03f3a34681..f207ad731f 100644 --- a/actionpack/test/template/render_test.rb +++ b/actionpack/test/template/render_test.rb @@ -126,6 +126,13 @@ module RenderTestCases assert_equal "only partial", @view.render("test/partial_only") end + def test_render_outside_path + assert File.exist?(File.join(File.dirname(__FILE__), '../../test/abstract_unit.rb')) + assert_raises ActionView::MissingTemplate do + @view.render(:template => "../\\../test/abstract_unit.rb") + end + end + def test_render_partial assert_equal "only partial", @view.render(:partial => "test/partial_only") end |