Attachment 1131947 Details for Bug 1310043 – Upstream patch for 4.1

[patch] Upstream patch for 4.1

4-1-render_data_leak_2.patch (text/plain), 9.22 KB, created by Adam Mariš on 2016-03-01 14:11:11 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Adam Mariš

Created: 2016-03-01 14:11:11 UTC

Size: 9.22 KB

patch

obsolete

>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 +++++++++++++
> actionview/CHANGELOG.md                            | 10 ++++++++
> actionview/lib/action_view/rendering.rb            |  4 +--
> .../test/actionpack/controller/render_test.rb      | 23 ++++-------------
> 6 files changed, 43 insertions(+), 49 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 @@ class ExpiresInRenderTest < ActionController::TestCase
>       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
>     assert File.exist?(File.join(File.dirname(__FILE__), '../../test/abstract_unit.rb'))
>     assert_raises ActionView::MissingTemplate do
>diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md
>index 80a2a5e..05bda0d 100644
>--- a/actionview/CHANGELOG.md
>+++ b/actionview/CHANGELOG.md
>@@ -1,3 +1,13 @@
>+*   Changed the meaning of `render "foo/bar"`.
>+
>+    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.
>+
>+    *Eileen Uchitelle*
>+
>+
> ## Rails 4.1.14 (November 12, 2015) ##
> 
> *   Fix `mail_to` when called with `nil` as argument.
>diff --git a/actionview/lib/action_view/rendering.rb b/actionview/lib/action_view/rendering.rb
>index 017302d..6283830 100644
>--- a/actionview/lib/action_view/rendering.rb
>+++ b/actionview/lib/action_view/rendering.rb
>@@ -107,7 +107,7 @@ module ActionView
>       end
> 
>       # Normalize args by converting render "foo" to render :action => "foo" and
>-      # render "foo/bar" to render :file => "foo/bar".
>+      # render "foo/bar" to render :template => "foo/bar".
>       # :api: private
>       def _normalize_args(action=nil, options={})
>         options = super(action, options)
>@@ -117,7 +117,7 @@ module ActionView
>           options = action
>         when String, Symbol
>           action = action.to_s
>-          key = action.include?(?/) ? :file : :action
>+          key = action.include?(?/) ? :template : :action
>           options[key] = action
>         else
>           options[:partial] = action
>diff --git a/actionview/test/actionpack/controller/render_test.rb b/actionview/test/actionpack/controller/render_test.rb
>index 45b8049..a9991fe 100644
>--- a/actionview/test/actionpack/controller/render_test.rb
>+++ b/actionview/test/actionpack/controller/render_test.rb
>@@ -91,17 +91,17 @@ class TestController < ApplicationController
> 
>   # :ported:
>   def render_hello_world
>-    render :template => "test/hello_world"
>+    render "test/hello_world"
>   end
> 
>   def render_hello_world_with_last_modified_set
>     response.last_modified = Date.new(2008, 10, 10).to_time
>-    render :template => "test/hello_world"
>+    render "test/hello_world"
>   end
> 
>   # :ported: compatibility
>   def render_hello_world_with_forward_slash
>-    render :template => "/test/hello_world"
>+    render "/test/hello_world"
>   end
> 
>   # :ported:
>@@ -111,7 +111,7 @@ class TestController < ApplicationController
> 
>   # :deprecated:
>   def render_template_in_top_directory_with_slash
>-    render :template => '/shared'
>+    render '/shared'
>   end
> 
>   # :ported:
>@@ -160,13 +160,6 @@ class TestController < ApplicationController
>   end
> 
>   # :ported:
>-  def render_file_as_string_with_instance_variables
>-    @secret = 'in the sauce'
>-    path = File.expand_path(File.join(File.dirname(__FILE__), '../../fixtures/test/render_file_with_ivar'))
>-    render path
>-  end
>-
>-  # :ported:
>   def render_file_not_using_full_path
>     @secret = 'in the sauce'
>     render :file => 'test/render_file_with_ivar'
>@@ -194,7 +187,7 @@ class TestController < ApplicationController
> 
>   def render_file_as_string_with_locals
>     path = File.expand_path(File.join(File.dirname(__FILE__), '../../fixtures/test/render_file_with_locals'))
>-    render path, :locals => {:secret => 'in the sauce'}
>+    render file: path, :locals => {:secret => 'in the sauce'}
>   end
> 
>   def accessing_request_in_template
>@@ -781,12 +774,6 @@ class RenderTest < ActionController::TestCase
>   end
> 
>   # :ported:
>-  def test_render_file_as_string_with_instance_variables
>-    get :render_file_as_string_with_instance_variables
>-    assert_equal "The secret is in the sauce\n", @response.body
>-  end
>-
>-  # :ported:
>   def test_render_file_not_using_full_path
>     get :render_file_not_using_full_path
>     assert_equal "The secret is in the sauce\n", @response.body
>-- 
>2.7.0
>

Actions: View | Diff

Attachments on bug 1310043: 1128503 | 1131946 | 1131947