From 1a65dd1c21cb7a70db054793deeb19dea1b357cf Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 26 Jan 2016 17:06:31 -0800 Subject: [PATCH 1/2] Change render "foo" to render a template and not a file. Previously, calling `render "foo/bar"` in a controller action is equivalent to `render file: "foo/bar"`. This has been changed to mean `render template: "foo/bar"` instead. If you need to render a file, please change your code to use the explicit form (`render file: "foo/bar"`) instead. Test that we are not allowing you to grab a file with an absolute path outside of your application directory. This is dangerous because it could be used to retrieve files from the server like `/etc/passwd`. Fix CVE-2016-2097. --- .../test/controller/new_base/render_file_test.rb | 29 ---------------------- .../controller/new_base/render_template_test.rb | 9 +++++++ actionpack/test/controller/render_test.rb | 17 +++++++++++++ 3 files changed, 26 insertions(+), 29 deletions(-) diff --git a/actionpack/test/controller/new_base/render_file_test.rb b/actionpack/test/controller/new_base/render_file_test.rb index a961cbf..0c21bb0 100644 --- a/actionpack/test/controller/new_base/render_file_test.rb +++ b/actionpack/test/controller/new_base/render_file_test.rb @@ -13,15 +13,6 @@ module RenderFile render :file => File.join(File.dirname(__FILE__), '../../fixtures/test/render_file_with_ivar') end - def without_file_key - render File.join(File.dirname(__FILE__), *%w[.. .. fixtures test hello_world]) - end - - def without_file_key_with_instance_variable - @secret = 'in the sauce' - render File.join(File.dirname(__FILE__), '../../fixtures/test/render_file_with_ivar') - end - def relative_path @secret = 'in the sauce' render :file => '../../fixtures/test/render_file_with_ivar' @@ -41,11 +32,6 @@ module RenderFile path = File.join(File.dirname(__FILE__), '../../fixtures/test/render_file_with_locals') render :file => path, :locals => {:secret => 'in the sauce'} end - - def without_file_key_with_locals - path = FIXTURES.join('test/render_file_with_locals').to_s - render path, :locals => {:secret => 'in the sauce'} - end end class TestBasic < Rack::TestCase @@ -61,16 +47,6 @@ module RenderFile assert_response "The secret is in the sauce\n" end - test "rendering path without specifying the :file key" do - get :without_file_key - assert_response "Hello world!" - end - - test "rendering path without specifying the :file key with ivar" do - get :without_file_key_with_instance_variable - assert_response "The secret is in the sauce\n" - end - test "rendering a relative path" do get :relative_path assert_response "The secret is in the sauce\n" @@ -90,10 +66,5 @@ module RenderFile get :with_locals assert_response "The secret is in the sauce\n" end - - test "rendering path without specifying the :file key with locals" do - get :without_file_key_with_locals - assert_response "The secret is in the sauce\n" - end end end diff --git a/actionpack/test/controller/new_base/render_template_test.rb b/actionpack/test/controller/new_base/render_template_test.rb index b7a9cf9..b0c4efb 100644 --- a/actionpack/test/controller/new_base/render_template_test.rb +++ b/actionpack/test/controller/new_base/render_template_test.rb @@ -45,6 +45,10 @@ module RenderTemplate render :template => "locals", :locals => { :secret => 'area51' } end + def with_locals_without_key + render "locals", :locals => { :secret => 'area51' } + end + def builder_template render :template => "xml_template" end @@ -101,6 +105,11 @@ module RenderTemplate assert_response "The secret is area51" end + test "rendering a template with local variables without key" do + get :with_locals + assert_response "The secret is area51" + end + test "rendering a builder template" do get :builder_template, "format" => "xml" assert_response "\n

Hello

\n\n" diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 17a019e..0fcbb86 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -261,6 +261,11 @@ end class ExpiresInRenderTest < ActionController::TestCase tests TestController + def setup + super + ActionController::Base.view_paths.paths.each(&:clear_cache) + 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')) @@ -269,6 +274,18 @@ response.body end + def test_dynamic_render_with_absolute_path + file = Tempfile.new('name') + file.write "secrets!" + file.flush + assert_raises ActionView::MissingTemplate do + get :dynamic_render, { id: file.path } + end + ensure + file.close + file.unlink + end + def test_dynamic_render_file_hash assert_raises ArgumentError do get :dynamic_render, { id: { file: '../\\../test/abstract_unit.rb' } } -- 2.7.0