From 1a65dd1c21cb7a70db054793deeb19dea1b357cf Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
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 "<html>\n <p>Hello</p>\n</html>\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