From 5c656a271a890cca4b3d438cc1fc76ff98011cbe Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 20 Jan 2016 10:39:19 -0800 Subject: [PATCH] allow :file to be outside rails root, but anything else must be inside the rails view directory Conflicts: actionpack/test/controller/render_test.rb actionview/lib/action_view/template/resolver.rb CVE-2016-0752 --- actionpack/lib/abstract_controller/rendering.rb | 8 +++++- actionpack/test/controller/render_test.rb | 31 ++++++++++++++++++++++ actionview/lib/action_view/lookup_context.rb | 4 +++ actionview/lib/action_view/path_set.rb | 26 +++++++++++++----- .../lib/action_view/renderer/abstract_renderer.rb | 2 +- .../lib/action_view/renderer/template_renderer.rb | 2 +- actionview/lib/action_view/template/resolver.rb | 25 ++++++++++++++--- actionview/lib/action_view/testing/resolvers.rb | 4 +-- actionview/test/template/render_test.rb | 7 +++++ 9 files changed, 93 insertions(+), 16 deletions(-) diff --git a/actionpack/lib/abstract_controller/rendering.rb b/actionpack/lib/abstract_controller/rendering.rb index 9d10140..e80d97f 100644 --- a/actionpack/lib/abstract_controller/rendering.rb +++ b/actionpack/lib/abstract_controller/rendering.rb @@ -77,7 +77,13 @@ module AbstractController # render "foo/bar" to render :file => "foo/bar". # :api: plugin def _normalize_args(action=nil, options={}) - if action.is_a? Hash + case action + when ActionController::Parameters + unless action.permitted? + raise ArgumentError, "render parameters are not permitted" + end + action + when Hash action else options diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 26806fb..17a019e 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -52,6 +52,16 @@ class TestController < ActionController::Base end end + def dynamic_render + render params[:id] # => String, AC:Params + end + + def dynamic_render_with_file + # This is extremely bad, but should be possible to do. + file = params[:id] # => String, AC:Params + render file: file + 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' @@ -251,6 +261,20 @@ end class ExpiresInRenderTest < ActionController::TestCase tests TestController + 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')) + response = get :dynamic_render_with_file, { id: '../\\../test/abstract_unit.rb' } + assert_equal File.read(File.join(File.dirname(__FILE__), '../../test/abstract_unit.rb')), + response.body + end + + def test_dynamic_render_file_hash + assert_raises ArgumentError do + get :dynamic_render, { id: { file: '../\\../test/abstract_unit.rb' } } + end + end + def test_expires_in_header get :conditional_hello_with_expires_in assert_equal "max-age=60, private", @response.headers["Cache-Control"] -- 2.2.1