Attachment 1131939 Details for Bug 1310054 – Upstream patch 3.2

[patch] Upstream patch 3.2

3-2-secure_inline_with_params1.patch (text/plain), 5.34 KB, created by Adam Mariš on 2016-03-01 13:51:09 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Adam Mariš

Created: 2016-03-01 13:51:09 UTC

Size: 5.34 KB

patch

obsolete

>From 1f2eb59c9858b35564cf8e8508abdf63321a8d61 Mon Sep 17 00:00:00 2001
>From: Arthur Neves <arthurnn@gmail.com>
>Date: Tue, 2 Feb 2016 12:34:11 -0500
>Subject: [PATCH 2/2] Don't allow render(params) in view/controller
>
>`render(params)` is dangerous and could be a vector for attackers.
>
>Don't allow calls to render passing params on views or controllers.
>
>On a controller or view, we should not allow something like `render
>params[:id]` or `render params`.
>That could be problematic, because an attacker could pass input that
>could lead to a remote code execution attack.
>
>This patch is also compatible when using strong parameters.
>
>CVE-2016-2098
>---
> actionpack/lib/action_view/renderer/renderer.rb |  5 +++
> actionpack/test/controller/render_test.rb       | 53 ++++++++++++++++++++++---
> actionpack/test/template/render_test.rb         | 27 +++++++++++++
> 3 files changed, 79 insertions(+), 6 deletions(-)
>
>diff --git a/actionpack/lib/action_view/renderer/renderer.rb b/actionpack/lib/action_view/renderer/renderer.rb
>index bf1b5a7..0f35989 100644
>--- a/actionpack/lib/action_view/renderer/renderer.rb
>+++ b/actionpack/lib/action_view/renderer/renderer.rb
>@@ -11,6 +11,11 @@ module ActionView
> 
>     # Main render entry point shared by AV and AC.
>     def render(context, options)
>+      if (options.is_a?(HashWithIndifferentAccess) && !options.respond_to?(:permitted?)) ||
>+         (options.respond_to?(:permitted?) && !options.permitted?)
>+        raise ArgumentError, "render parameters are not permitted"
>+      end
>+
>       if options.key?(:partial)
>         render_partial(context, options)
>       else
>diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb
>index bc42c66..de80b78 100644
>--- a/actionpack/test/controller/render_test.rb
>+++ b/actionpack/test/controller/render_test.rb
>@@ -70,6 +70,10 @@ class TestController < ActionController::Base
>     render :file => file
>   end
> 
>+  def dynamic_inline_render
>+    render :inline => "<%= render(params) %>"
>+  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'
>@@ -245,12 +249,6 @@ class TestController < ActionController::Base
>     render :inline =>  "<%= action_name %>"
>   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
>@@ -742,6 +740,15 @@ class MetalTestController < ActionController::Metal
>   end
> end
> 
>+class MetalWithoutAVTestController < ActionController::Metal
>+  include AbstractController::Rendering
>+  include ActionController::Rendering
>+
>+  def dynamic_params_render
>+    render params
>+  end
>+end
>+
> class RenderTest < ActionController::TestCase
>   tests TestController
> 
>@@ -783,6 +790,29 @@ class RenderTest < ActionController::TestCase
>     end
>   end
> 
>+  def test_dynamic_render_file_hash
>+    assert_raises ArgumentError do
>+      get :dynamic_render, { :id => { :file => '../\\../test/abstract_unit.rb' } }
>+    end
>+  end
>+
>+  def test_dynamic_inline
>+    assert_raises ArgumentError do
>+      get :dynamic_render, { :id => { :inline => '<%= RUBY_VERSION %>' } }
>+    end
>+  end
>+
>+  def test_dynamic_render_on_view
>+    file = Tempfile.new('_name')
>+    file.write "secrets!"
>+    file.flush
>+
>+    e = assert_raises ActionView::Template::Error do
>+      get :dynamic_inline_render, { :file => file.path }
>+    end
>+    assert_equal "render parameters are not permitted", e.message
>+  end
>+
>   # :ported:
>   def test_simple_show
>     get :hello_world
>@@ -1657,3 +1687,14 @@ class MetalRenderTest < ActionController::TestCase
>     assert_equal "NilClass", @response.body
>   end
> end
>+
>+class MetalRenderWithoutAVTest < ActionController::TestCase
>+  tests MetalWithoutAVTestController
>+
>+  def test_dynamic_params_render
>+    e = assert_raises ArgumentError do
>+      get :dynamic_params_render, { inline: '<%= RUBY_VERSION %>' }
>+    end
>+    assert_equal "render parameters are not permitted", e.message
>+  end
>+end
>diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb
>index f207ad7..2d04665 100644
>--- a/actionpack/test/template/render_test.rb
>+++ b/actionpack/test/template/render_test.rb
>@@ -133,6 +133,33 @@ module RenderTestCases
>     end
>   end
> 
>+  def test_render_with_params
>+    params = { :inline => '<%= RUBY_VERSION %>' }.with_indifferent_access
>+    assert_raises ArgumentError do
>+      @view.render(params)
>+    end
>+  end
>+
>+  def test_render_with_strong_parameters
>+    # compatibility with Strong Parameters gem
>+    params = Class.new(HashWithIndifferentAccess).new
>+    params[:inline] = '<%= RUBY_VERSION %>'
>+    e = assert_raises ArgumentError do
>+      @view.render(params)
>+    end
>+    assert_equal "render parameters are not permitted", e.message
>+  end
>+
>+  def test_render_with_permitted_strong_parameters
>+    # compatibility with Strong Parameters gem
>+    params = Class.new(HashWithIndifferentAccess).new
>+    params[:inline] = "<%= 'hello' %>"
>+    def params.permitted?
>+      true
>+    end
>+    assert_equal 'hello', @view.render(params)
>+  end
>+
>   def test_render_partial
>     assert_equal "only partial", @view.render(:partial => "test/partial_only")
>   end
>-- 
>2.5.4 (Apple Git-61)
>

Actions: View | Diff

Attachments on bug 1310054: 1128508 | 1131939 | 1131942 | 1131943