aboutsummaryrefslogtreecommitdiffstats
path: root/actionpack
diff options
context:
space:
mode:
authorArthur Neves <arthurnn@gmail.com>2016-02-02 12:34:11 -0500
committerRafael Mendonça França <rafaelmfranca@gmail.com>2016-02-29 15:39:02 -0300
commitaf9b9132f82d1f468836997c716a02f14e61c38c (patch)
tree206e4b89c5486826efdd5223f17a8767e047cc48 /actionpack
parent9892626579d1c62c367e5344a1d1642708340f88 (diff)
downloadrails-af9b9132f82d1f468836997c716a02f14e61c38c.tar.gz
rails-af9b9132f82d1f468836997c716a02f14e61c38c.tar.bz2
rails-af9b9132f82d1f468836997c716a02f14e61c38c.zip
Complete work on 3.2 for render_data_leak patch.
Render could leak access to external files before this patch. A previous patch(CVE-2016-0752), attempted to fix this. However the tests were miss-placed outside the TestCase subclass, so they were not running. We should allow :file to be outside rails root, but anything else must be inside the rails view directory. The implementation has changed a bit though. Now the patch is more similar with the 4.x series patches. Now `render 'foo/bar'`, will add a special key in the options hash, and not use the :file one, so when we look up that file, we don't set the fallbacks, and only lookup a template, to constraint the folders that can be accessed. CVE-2016-2097
Diffstat (limited to 'actionpack')
-rw-r--r--actionpack/lib/abstract_controller/rendering.rb3
-rw-r--r--actionpack/lib/action_view/lookup_context.rb4
-rw-r--r--actionpack/lib/action_view/path_set.rb26
-rw-r--r--actionpack/lib/action_view/renderer/abstract_renderer.rb2
-rw-r--r--actionpack/lib/action_view/renderer/template_renderer.rb5
-rw-r--r--actionpack/lib/action_view/template/resolver.rb32
-rw-r--r--actionpack/lib/action_view/testing/resolvers.rb5
-rw-r--r--actionpack/test/controller/new_base/render_file_test.rb49
-rw-r--r--actionpack/test/controller/render_test.rb56
9 files changed, 79 insertions, 103 deletions
diff --git a/actionpack/lib/abstract_controller/rendering.rb b/actionpack/lib/abstract_controller/rendering.rb
index f74fd6ac4a..5b45a13269 100644
--- a/actionpack/lib/abstract_controller/rendering.rb
+++ b/actionpack/lib/abstract_controller/rendering.rb
@@ -147,12 +147,11 @@ module AbstractController
options = action
when String, Symbol
action = action.to_s
- key = action.include?(?/) ? :file : :action
+ key = action.include?(?/) ? :app_template_file : :action
options[key] = action
else
options[:partial] = action
end
-
options
end
diff --git a/actionpack/lib/action_view/lookup_context.rb b/actionpack/lib/action_view/lookup_context.rb
index 9331d13577..91ca4a7c93 100644
--- a/actionpack/lib/action_view/lookup_context.rb
+++ b/actionpack/lib/action_view/lookup_context.rb
@@ -127,6 +127,10 @@ module ActionView
@view_paths.find_all(*args_for_lookup(name, prefixes, partial, keys, options))
end
+ def find_file(name, prefixes = [], partial = false, keys = [], options = {})
+ @view_paths.find_file(*args_for_lookup(name, prefixes, partial, keys, options))
+ end
+
def exists?(name, prefixes = [], partial = false, keys = [], options = {})
@view_paths.exists?(*args_for_lookup(name, prefixes, partial, keys, options))
end
diff --git a/actionpack/lib/action_view/path_set.rb b/actionpack/lib/action_view/path_set.rb
index bbb1af8154..1c54a970cb 100644
--- a/actionpack/lib/action_view/path_set.rb
+++ b/actionpack/lib/action_view/path_set.rb
@@ -58,23 +58,35 @@ module ActionView #:nodoc:
find_all(*args).first || raise(MissingTemplate.new(self, *args))
end
+ def find_file(path, prefixes = [], *args)
+ _find_all(path, prefixes, args, true).first || raise(MissingTemplate.new(self, path, prefixes, *args))
+ end
+
def find_all(path, prefixes = [], *args)
+ _find_all path, prefixes, args, false
+ end
+
+ def exists?(path, prefixes, *args)
+ find_all(path, prefixes, *args).any?
+ end
+
+ private
+
+ def _find_all(path, prefixes, args, outside_app)
prefixes = [prefixes] if String === prefixes
prefixes.each do |prefix|
paths.each do |resolver|
- templates = resolver.find_all(path, prefix, *args)
+ if outside_app
+ templates = resolver.find_all_anywhere(path, prefix, *args)
+ else
+ templates = resolver.find_all(path, prefix, *args)
+ end
return templates unless templates.empty?
end
end
[]
end
- def exists?(path, prefixes, *args)
- find_all(path, prefixes, *args).any?
- end
-
- private
-
def typecast(paths)
paths.map do |path|
case path
diff --git a/actionpack/lib/action_view/renderer/abstract_renderer.rb b/actionpack/lib/action_view/renderer/abstract_renderer.rb
index 0b5d3785d4..021fb198a9 100644
--- a/actionpack/lib/action_view/renderer/abstract_renderer.rb
+++ b/actionpack/lib/action_view/renderer/abstract_renderer.rb
@@ -1,6 +1,6 @@
module ActionView
class AbstractRenderer #:nodoc:
- delegate :find_template, :template_exists?, :with_fallbacks, :update_details,
+ delegate :find_template, :find_file, :template_exists?, :with_fallbacks, :update_details,
:with_layout_format, :formats, :to => :@lookup_context
def initialize(lookup_context)
diff --git a/actionpack/lib/action_view/renderer/template_renderer.rb b/actionpack/lib/action_view/renderer/template_renderer.rb
index a27d5dd1b1..578db362f6 100644
--- a/actionpack/lib/action_view/renderer/template_renderer.rb
+++ b/actionpack/lib/action_view/renderer/template_renderer.rb
@@ -21,11 +21,12 @@ module ActionView
# Determine the template to be rendered using the given options.
def determine_template(options) #:nodoc:
keys = options[:locals].try(:keys) || []
-
if options.key?(:text)
Template::Text.new(options[:text], formats.try(:first))
+ elsif options.key?(:app_template_file)
+ find_template(options[:app_template_file], nil, false, keys, @details)
elsif options.key?(:file)
- with_fallbacks { find_template(options[:file], nil, false, keys, @details) }
+ with_fallbacks { find_file(options[:file], nil, false, keys, @details) }
elsif options.key?(:inline)
handler = Template.handler_for_extension(options[:type] || "erb")
Template.new(options[:inline], "inline template", handler, :locals => keys)
diff --git a/actionpack/lib/action_view/template/resolver.rb b/actionpack/lib/action_view/template/resolver.rb
index d3a6d1a3b8..bea8c221c3 100644
--- a/actionpack/lib/action_view/template/resolver.rb
+++ b/actionpack/lib/action_view/template/resolver.rb
@@ -43,7 +43,13 @@ module ActionView
# Normalizes the arguments and passes it on to find_template.
def find_all(name, prefix=nil, partial=false, details={}, key=nil, locals=[])
cached(key, [name, prefix, partial], details, locals) do
- find_templates(name, prefix, partial, details)
+ find_templates(name, prefix, partial, details, false)
+ end
+ end
+
+ def find_all_anywhere(name, prefix, partial=false, details={}, key=nil, locals=[])
+ cached(key, [name, prefix, partial], details, locals) do
+ find_templates(name, prefix, partial, details, true)
end
end
@@ -54,8 +60,8 @@ module ActionView
# This is what child classes implement. No defaults are needed
# because Resolver guarantees that the arguments are present and
# normalized.
- def find_templates(name, prefix, partial, details)
- raise NotImplementedError, "Subclasses must implement a find_templates(name, prefix, partial, details) method"
+ def find_templates(name, prefix, partial, details, outside_app_allowed = false)
+ raise NotImplementedError, "Subclasses must implement a find_templates(name, prefix, partial, details, outside_app_allowed) method"
end
# Helpers that builds a path. Useful for building virtual paths.
@@ -110,24 +116,21 @@ module ActionView
super()
end
- cattr_accessor :allow_external_files, :instance_reader => false, :instance_writer => false
- self.allow_external_files = false
+ cattr_accessor :instance_reader => false, :instance_writer => false
private
- def find_templates(name, prefix, partial, details)
+ def find_templates(name, prefix, partial, details, outside_app_allowed = false)
path = Path.build(name, prefix, partial)
- query(path, details, details[:formats])
+ query(path, details, details[:formats], outside_app_allowed)
end
- def query(path, details, formats)
+ def query(path, details, formats, outside_app_allowed)
query = build_query(path, details)
template_paths = find_template_paths query
- unless self.class.allow_external_files
- template_paths = reject_files_external_to_app(template_paths)
- end
+ template_paths = reject_files_external_to_app(template_paths) unless outside_app_allowed
template_paths.map { |template|
handler, format = extract_handler_and_format(template, formats)
@@ -267,7 +270,12 @@ module ActionView
class OptimizedFileSystemResolver < FileSystemResolver #:nodoc:
def build_query(path, details)
exts = EXTENSIONS.map { |ext| details[ext] }
- query = escape_entry(File.join(@path, path))
+
+ if path.to_s.starts_with? @path.to_s
+ query = escape_entry(path)
+ else
+ query = escape_entry(File.join(@path, path))
+ end
query + exts.map { |ext|
"{#{ext.compact.uniq.map { |e| ".#{e}," }.join}}"
diff --git a/actionpack/lib/action_view/testing/resolvers.rb b/actionpack/lib/action_view/testing/resolvers.rb
index 7afa2fa613..d552151a1d 100644
--- a/actionpack/lib/action_view/testing/resolvers.rb
+++ b/actionpack/lib/action_view/testing/resolvers.rb
@@ -19,7 +19,7 @@ module ActionView #:nodoc:
private
- def query(path, exts, formats)
+ def query(path, exts, formats, outside_app_allowed)
query = ""
EXTENSIONS.each do |ext|
query << '(' << exts[ext].map {|e| e && Regexp.escape(".#{e}") }.join('|') << '|)'
@@ -40,11 +40,10 @@ module ActionView #:nodoc:
end
class NullResolver < PathResolver
- def query(path, exts, formats)
+ def query(path, exts, formats, outside_app_allowed)
handler, format = extract_handler_and_format(path, formats)
[ActionView::Template.new("Template generated by Null Resolver", path, handler, :virtual_path => path, :format => format)]
end
end
end
-
diff --git a/actionpack/test/controller/new_base/render_file_test.rb b/actionpack/test/controller/new_base/render_file_test.rb
index c0e23db457..3fdf7a67c0 100644
--- a/actionpack/test/controller/new_base/render_file_test.rb
+++ b/actionpack/test/controller/new_base/render_file_test.rb
@@ -13,15 +13,6 @@ module RenderFile
render :file => File.join(File.dirname(__FILE__), '../../fixtures/test/render_file_with_ivar')
end
- def without_file_key
- render File.join(File.dirname(__FILE__), *%w[.. .. fixtures test hello_world])
- end
-
- def without_file_key_with_instance_variable
- @secret = 'in the sauce'
- render File.join(File.dirname(__FILE__), '../../fixtures/test/render_file_with_ivar')
- end
-
def relative_path
@secret = 'in the sauce'
render :file => '../../fixtures/test/render_file_with_ivar'
@@ -41,11 +32,6 @@ module RenderFile
path = File.join(File.dirname(__FILE__), '../../fixtures/test/render_file_with_locals')
render :file => path, :locals => {:secret => 'in the sauce'}
end
-
- def without_file_key_with_locals
- path = FIXTURES.join('test/render_file_with_locals').to_s
- render path, :locals => {:secret => 'in the sauce'}
- end
end
class TestBasic < Rack::TestCase
@@ -61,36 +47,6 @@ module RenderFile
assert_response "The secret is in the sauce\n"
end
- test "rendering path without specifying the :file key" do
- get :without_file_key
- assert_response "Hello world!"
- end
-
- test "rendering path without specifying the :file key with ivar" do
- get :without_file_key_with_instance_variable
- assert_response "The secret is in the sauce\n"
- end
-
- test "rendering a relative path" do
- 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
- 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
get :pathname
assert_response "The secret is in the sauce\n"
@@ -100,10 +56,5 @@ module RenderFile
get :with_locals
assert_response "The secret is in the sauce\n"
end
-
- test "rendering path without specifying the :file key with locals" do
- get :without_file_key_with_locals
- assert_response "The secret is in the sauce\n"
- end
end
end
diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb
index 355523293f..bc42c6614a 100644
--- a/actionpack/test/controller/render_test.rb
+++ b/actionpack/test/controller/render_test.rb
@@ -245,33 +245,6 @@ 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_with_absolute_path
- file = Tempfile.new('name')
- file.write "secrets!"
- file.flush
- assert_raises ActionView::MissingTemplate do
- response = get :dynamic_render, { :id => file.path }
- end
- ensure
- file.close
- file.unlink
- 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' } }
@@ -779,6 +752,35 @@ class RenderTest < ActionController::TestCase
@controller.logger = Logger.new(nil)
@request.host = "www.nextangle.com"
+ ActionController::Base.view_paths.paths.each(&:clear_cache)
+ 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
+
+ # :ported:
+ def test_dynamic_render_with_absolute_path
+ file = Tempfile.new('name')
+ file.write "secrets!"
+ file.flush
+ assert_raises ActionView::MissingTemplate do
+ get :dynamic_render, { :id => file.path }
+ end
+ ensure
+ file.close
+ file.unlink
+ 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
# :ported: