From 769b4d3f6638f8871bb7ca7ad3d076a3dcc9e1a9 Mon Sep 17 00:00:00 2001
From: Arthur Neves <arthurnn@gmail.com>
Date: Tue, 2 Feb 2016 12:34:11 -0500
Subject: 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(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_view/renderer/renderer.rb b/actionpack/lib/action_view/renderer/renderer.rb
index bf1b5a7d22..0f359899d6 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 bc42c6614a..de80b78272 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 f207ad731f..2d0466585f 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
-- 
cgit v1.2.3