aboutsummaryrefslogtreecommitdiffstats
path: root/actionpack
diff options
context:
space:
mode:
authorAaron Patterson <aaron.patterson@gmail.com>2016-01-20 10:39:19 -0800
committerAaron Patterson <aaron.patterson@gmail.com>2016-01-22 15:01:49 -0800
commit18269d250fa58001ce7d8318571546aa90412975 (patch)
treebb44a4b20c0964b201d38ed864f7ad6b19b3fb60 /actionpack
parentcdabc95608336dbea7b6a3a3e925de5bbd5313ba (diff)
downloadrails-18269d250fa58001ce7d8318571546aa90412975.tar.gz
rails-18269d250fa58001ce7d8318571546aa90412975.tar.bz2
rails-18269d250fa58001ce7d8318571546aa90412975.zip
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
Diffstat (limited to 'actionpack')
-rw-r--r--actionpack/lib/action_view/template/resolver.rb17
-rw-r--r--actionpack/test/controller/new_base/render_file_test.rb18
-rw-r--r--actionpack/test/controller/render_test.rb31
-rw-r--r--actionpack/test/template/render_test.rb7
4 files changed, 69 insertions, 4 deletions
diff --git a/actionpack/lib/action_view/template/resolver.rb b/actionpack/lib/action_view/template/resolver.rb
index 47ea8a3c9b..c6db6685e4 100644
--- a/actionpack/lib/action_view/template/resolver.rb
+++ b/actionpack/lib/action_view/template/resolver.rb
@@ -110,6 +110,9 @@ module ActionView
super()
end
+ cattr_accessor :allow_external_files, instance_reader: false, instance_writer: false
+ self.allow_external_files = false
+
private
def find_templates(name, prefix, partial, details)
@@ -122,6 +125,10 @@ module ActionView
template_paths = find_template_paths query
+ unless self.class.allow_external_files
+ template_paths = reject_files_external_to_app(template_paths)
+ end
+
template_paths.map { |template|
handler, format = extract_handler_and_format(template, formats)
contents = File.binread template
@@ -133,6 +140,10 @@ module ActionView
}
end
+ def reject_files_external_to_app(files)
+ files.reject { |filename| !inside_path?(@path, filename) }
+ end
+
if RUBY_VERSION >= '2.2.0'
def find_template_paths(query)
Dir[query].reject { |filename|
@@ -153,6 +164,12 @@ module ActionView
end
end
+ def inside_path?(path, filename)
+ filename = File.expand_path(filename)
+ path = File.join(path, '')
+ filename.start_with?(path)
+ end
+
# Helper for building query glob string based on resolver's pattern.
def build_query(path, details)
query = @pattern.dup
diff --git a/actionpack/test/controller/new_base/render_file_test.rb b/actionpack/test/controller/new_base/render_file_test.rb
index a961cbf849..c0e23db457 100644
--- a/actionpack/test/controller/new_base/render_file_test.rb
+++ b/actionpack/test/controller/new_base/render_file_test.rb
@@ -72,13 +72,23 @@ module RenderFile
end
test "rendering a relative path" do
- get :relative_path
- assert_response "The secret is in the sauce\n"
+ begin
+ ActionView::PathResolver.allow_external_files = true
+ get :relative_path
+ assert_response "The secret is in the sauce\n"
+ ensure
+ ActionView::PathResolver.allow_external_files = false
+ end
end
test "rendering a relative path with dot" do
- get :relative_path_with_dot
- assert_response "The secret is in the sauce\n"
+ begin
+ ActionView::PathResolver.allow_external_files = true
+ get :relative_path_with_dot
+ assert_response "The secret is in the sauce\n"
+ ensure
+ ActionView::PathResolver.allow_external_files = false
+ end
end
test "rendering a Pathname" do
diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb
index 3964540def..6210524cef 100644
--- a/actionpack/test/controller/render_test.rb
+++ b/actionpack/test/controller/render_test.rb
@@ -60,6 +60,16 @@ class TestController < ActionController::Base
end
end
+ def dynamic_render
+ render params[:id] # => String, Hash
+ end
+
+ def dynamic_render_with_file
+ # This is extremely bad, but should be possible to do.
+ file = params[:id] # => String, Hash
+ 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'
@@ -235,6 +245,27 @@ class TestController < ActionController::Base
render :inline => "<%= action_name %>"
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'))
+ 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
+ assert File.exist?(File.join(File.dirname(__FILE__), '../../test/abstract_unit.rb'))
+ assert_raises ActionView::MissingTemplate do
+ get :dynamic_render, { id: '../\\../test/abstract_unit.rb' }
+ end
+ 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
diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb
index 03f3a34681..f207ad731f 100644
--- a/actionpack/test/template/render_test.rb
+++ b/actionpack/test/template/render_test.rb
@@ -126,6 +126,13 @@ module RenderTestCases
assert_equal "only partial", @view.render("test/partial_only")
end
+ def test_render_outside_path
+ assert File.exist?(File.join(File.dirname(__FILE__), '../../test/abstract_unit.rb'))
+ assert_raises ActionView::MissingTemplate do
+ @view.render(:template => "../\\../test/abstract_unit.rb")
+ end
+ end
+
def test_render_partial
assert_equal "only partial", @view.render(:partial => "test/partial_only")
end