aboutsummaryrefslogtreecommitdiffstats
path: root/actionview
diff options
context:
space:
mode:
authorJohn Hawthorn <john@hawthorn.email>2019-04-01 16:35:07 -0700
committerJohn Hawthorn <john@hawthorn.email>2019-04-03 09:02:28 -0700
commiteb52904eb5c19ab4e8ff7a7d4f501fe5e1142ad0 (patch)
tree69e7486d27a6c0555473cea655cc4ab36c9f6c1a /actionview
parentbeb0bc9907a31d0cbd2ca68c79c57a9e375761e8 (diff)
downloadrails-eb52904eb5c19ab4e8ff7a7d4f501fe5e1142ad0.tar.gz
rails-eb52904eb5c19ab4e8ff7a7d4f501fe5e1142ad0.tar.bz2
rails-eb52904eb5c19ab4e8ff7a7d4f501fe5e1142ad0.zip
Always reject files external to app
Previously, when using `render file:`, it was possible to render files not only at an absolute path or relative to the current directory, but relative to ANY view paths. This was probably done for absolutely maximum compatibility when addressing CVE-2016-0752, but I think is unlikely to be used in practice. Tihs commit removes the ability to `render file:` with a path relative to a non-fallback view path. Make FallbackResolver.new private To ensure nobody is making FallbackResolvers other than "/" and "". Make reject_files_external_... no-op for fallbacks Because there are only two values used for path: "" and "/", and File.join("", "") == File.join("/", "") == "/", this method was only testing that the absolute paths started at "/" (which of course all do). This commit doesn't change any behaviour, but it makes it explicit that the FallbackFileSystemResolver works this way. Remove outside_app_allowed argument Deprecate find_all_anywhere This is now equivalent to find_all Remove outside_app argument Deprecate find_file for find Both LookupContext#find_file and PathSet#find_file are now equivalent to their respective #find methods.
Diffstat (limited to 'actionview')
-rw-r--r--actionview/lib/action_view/lookup_context.rb5
-rw-r--r--actionview/lib/action_view/path_set.rb15
-rw-r--r--actionview/lib/action_view/renderer/template_renderer.rb2
-rw-r--r--actionview/lib/action_view/template/resolver.rb29
-rw-r--r--actionview/lib/action_view/testing/resolvers.rb4
-rw-r--r--actionview/test/template/fallback_file_system_resolver_test.rb2
-rw-r--r--actionview/test/template/lookup_context_test.rb8
7 files changed, 30 insertions, 35 deletions
diff --git a/actionview/lib/action_view/lookup_context.rb b/actionview/lib/action_view/lookup_context.rb
index fd3d025cbf..b9a8cc129c 100644
--- a/actionview/lib/action_view/lookup_context.rb
+++ b/actionview/lib/action_view/lookup_context.rb
@@ -130,9 +130,8 @@ module ActionView
end
alias :find_template :find
- def find_file(name, prefixes = [], partial = false, keys = [], options = {})
- @view_paths.find_file(*args_for_lookup(name, prefixes, partial, keys, options))
- end
+ alias :find_file :find
+ deprecate :find_file
def find_all(name, prefixes = [], partial = false, keys = [], options = {})
@view_paths.find_all(*args_for_lookup(name, prefixes, partial, keys, options))
diff --git a/actionview/lib/action_view/path_set.rb b/actionview/lib/action_view/path_set.rb
index 691b53e2da..d54749d8e3 100644
--- a/actionview/lib/action_view/path_set.rb
+++ b/actionview/lib/action_view/path_set.rb
@@ -48,12 +48,11 @@ 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
+ alias :find_file :find
+ deprecate :find_file
def find_all(path, prefixes = [], *args)
- _find_all path, prefixes, args, false
+ _find_all path, prefixes, args
end
def exists?(path, prefixes, *args)
@@ -71,15 +70,11 @@ module ActionView #:nodoc:
private
- def _find_all(path, prefixes, args, outside_app)
+ def _find_all(path, prefixes, args)
prefixes = [prefixes] if String === prefixes
prefixes.each do |prefix|
paths.each do |resolver|
- if outside_app
- templates = resolver.find_all_anywhere(path, prefix, *args)
- else
- templates = resolver.find_all(path, prefix, *args)
- end
+ templates = resolver.find_all(path, prefix, *args)
return templates unless templates.empty?
end
end
diff --git a/actionview/lib/action_view/renderer/template_renderer.rb b/actionview/lib/action_view/renderer/template_renderer.rb
index 698f535b49..e30016a027 100644
--- a/actionview/lib/action_view/renderer/template_renderer.rb
+++ b/actionview/lib/action_view/renderer/template_renderer.rb
@@ -30,7 +30,7 @@ module ActionView
Template::File.new(options[:file])
else
ActiveSupport::Deprecation.warn "render file: should be given the absolute path to a file"
- @lookup_context.with_fallbacks.find_file(options[:file], nil, false, keys, @details)
+ @lookup_context.with_fallbacks.find_template(options[:file], nil, false, keys, @details)
end
elsif options.key?(:inline)
handler = Template.handler_for_extension(options[:type] || "erb")
diff --git a/actionview/lib/action_view/template/resolver.rb b/actionview/lib/action_view/template/resolver.rb
index 095e6cc3a1..d5cb3c823a 100644
--- a/actionview/lib/action_view/template/resolver.rb
+++ b/actionview/lib/action_view/template/resolver.rb
@@ -118,17 +118,12 @@ module ActionView
locals = locals.map(&:to_s).sort!.freeze
cached(key, [name, prefix, partial], details, locals) do
- find_templates(name, prefix, partial, details, false, locals)
+ find_templates(name, prefix, partial, details, locals)
end
end
- def find_all_anywhere(name, prefix, partial = false, details = {}, key = nil, locals = [])
- locals = locals.map(&:to_s).sort!.freeze
-
- cached(key, [name, prefix, partial], details, locals) do
- find_templates(name, prefix, partial, details, true, locals)
- end
- end
+ alias :find_all_anywhere :find_all
+ deprecate :find_all_anywhere
def find_all_with_query(query) # :nodoc:
@cache.cache_query(query) { find_template_paths(File.join(@path, query)) }
@@ -141,8 +136,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, outside_app_allowed = false, locals = [])
- raise NotImplementedError, "Subclasses must implement a find_templates(name, prefix, partial, details, outside_app_allowed = false, locals = []) method"
+ def find_templates(name, prefix, partial, details, locals = [])
+ raise NotImplementedError, "Subclasses must implement a find_templates(name, prefix, partial, details, locals = []) method"
end
# Handles templates caching. If a key is given and caching is on
@@ -179,14 +174,14 @@ module ActionView
private
- def find_templates(name, prefix, partial, details, outside_app_allowed = false, locals)
+ def find_templates(name, prefix, partial, details, locals)
path = Path.build(name, prefix, partial)
- query(path, details, details[:formats], outside_app_allowed, locals)
+ query(path, details, details[:formats], locals)
end
- def query(path, details, formats, outside_app_allowed, locals)
+ def query(path, details, formats, locals)
template_paths = find_template_paths_from_details(path, details)
- template_paths = reject_files_external_to_app(template_paths) unless outside_app_allowed
+ template_paths = reject_files_external_to_app(template_paths)
template_paths.map do |template|
build_template(template, path.virtual, locals)
@@ -360,6 +355,8 @@ module ActionView
# The same as FileSystemResolver but does not allow templates to store
# a virtual path since it is invalid for such resolvers.
class FallbackFileSystemResolver < FileSystemResolver #:nodoc:
+ private_class_method :new
+
def self.instances
[new(""), new("/")]
end
@@ -367,5 +364,9 @@ module ActionView
def build_template(template, virtual_path, locals)
super(template, nil, locals)
end
+
+ def reject_files_external_to_app(files)
+ files
+ end
end
end
diff --git a/actionview/lib/action_view/testing/resolvers.rb b/actionview/lib/action_view/testing/resolvers.rb
index a16dc0096e..11ae9f731c 100644
--- a/actionview/lib/action_view/testing/resolvers.rb
+++ b/actionview/lib/action_view/testing/resolvers.rb
@@ -23,7 +23,7 @@ module ActionView #:nodoc:
private
- def query(path, exts, _, _, locals)
+ def query(path, exts, _, locals)
query = +""
EXTENSIONS.each_key do |ext|
query << "(" << exts[ext].map { |e| e && Regexp.escape(".#{e}") }.join("|") << "|)"
@@ -47,7 +47,7 @@ module ActionView #:nodoc:
end
class NullResolver < PathResolver
- def query(path, exts, _, _, locals)
+ def query(path, exts, _, locals)
handler, format, variant = extract_handler_and_format_and_variant(path)
[ActionView::Template.new("Template generated by Null Resolver", path.virtual, handler, virtual_path: path.virtual, format: format, variant: variant, locals: locals)]
end
diff --git a/actionview/test/template/fallback_file_system_resolver_test.rb b/actionview/test/template/fallback_file_system_resolver_test.rb
index 304cdb8a03..fa770f3a15 100644
--- a/actionview/test/template/fallback_file_system_resolver_test.rb
+++ b/actionview/test/template/fallback_file_system_resolver_test.rb
@@ -4,7 +4,7 @@ require "abstract_unit"
class FallbackFileSystemResolverTest < ActiveSupport::TestCase
def setup
- @root_resolver = ActionView::FallbackFileSystemResolver.new("/")
+ @root_resolver = ActionView::FallbackFileSystemResolver.send(:new, "/")
end
def test_should_have_no_virtual_path
diff --git a/actionview/test/template/lookup_context_test.rb b/actionview/test/template/lookup_context_test.rb
index 3e357fe1a7..72e1f50fdf 100644
--- a/actionview/test/template/lookup_context_test.rb
+++ b/actionview/test/template/lookup_context_test.rb
@@ -143,16 +143,16 @@ class LookupContextTest < ActiveSupport::TestCase
assert_deprecated do
@lookup_context.with_fallbacks do
assert_equal 3, @lookup_context.view_paths.size
- assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.new("")
- assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.new("/")
+ assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.instances[0]
+ assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.instances[1]
end
end
@lookup_context = @lookup_context.with_fallbacks
assert_equal 3, @lookup_context.view_paths.size
- assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.new("")
- assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.new("/")
+ assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.instances[0]
+ assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.instances[1]
end
test "add fallbacks just once in nested fallbacks calls" do