Skip to content

Commit

Permalink
Don't allow render(params) in view/controller
Browse files Browse the repository at this point in the history
`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
  • Loading branch information
arthurnn authored and rafaelfranca committed Feb 29, 2016
1 parent af9b913 commit 769b4d3
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 6 deletions.
5 changes: 5 additions & 0 deletions actionpack/lib/action_view/renderer/renderer.rb
Expand Up @@ -11,6 +11,11 @@ def initialize(lookup_context)

# 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
Expand Down
53 changes: 47 additions & 6 deletions actionpack/test/controller/render_test.rb
Expand Up @@ -70,6 +70,10 @@ def dynamic_render_with_file
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'
Expand Down Expand Up @@ -245,12 +249,6 @@ def accessing_action_name_in_template
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
Expand Down Expand Up @@ -742,6 +740,15 @@ def accessing_logger_in_template
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

Expand Down Expand Up @@ -783,6 +790,29 @@ def test_dynamic_render
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
Expand Down Expand Up @@ -1657,3 +1687,14 @@ def test_access_to_logger_in_view
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
27 changes: 27 additions & 0 deletions actionpack/test/template/render_test.rb
Expand Up @@ -133,6 +133,33 @@ def test_render_outside_path
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
Expand Down

0 comments on commit 769b4d3

Please sign in to comment.