allow :file to be outside rails root, but anything else must be inside the rails view directory

CVE-2016-0752

9 files changed, 93 insertions, 16 deletions

diff --git a/actionpack/lib/abstract_controller/rendering.rb b/actionpack/lib/abstract_controller/rendering.rb
index a73f188623..63fd76d9b7 100644
--- a/actionpack/lib/abstract_controller/rendering.rb
+++ b/actionpack/lib/abstract_controller/rendering.rb
@@ -82,7 +82,13 @@ module AbstractController
     # <tt>render :file => "foo/bar"</tt>.
     # :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 256ebf6a07..bbc216b52f 100644
--- a/actionpack/test/controller/render_test.rb
+++ b/actionpack/test/controller/render_test.rb
@@ -62,6 +62,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
+
   class Collection
     def initialize(records)
       @records = records
@@ -243,6 +253,27 @@ 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, params: { 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, params: { id: '../\\../test/abstract_unit.rb' }
+    end
+  end
+
+  def test_dynamic_render_file_hash
+    assert_raises ArgumentError do
+      get :dynamic_render, params: { 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"]
diff --git a/actionview/lib/action_view/lookup_context.rb b/actionview/lib/action_view/lookup_context.rb
index d3935788ef..fadb0b549f 100644
--- a/actionview/lib/action_view/lookup_context.rb
+++ b/actionview/lib/action_view/lookup_context.rb
@@ -123,6 +123,10 @@ 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
+
       def find_all(name, prefixes = [], partial = false, keys = [], options = {})
         @view_paths.find_all(*args_for_lookup(name, prefixes, partial, keys, options))
       end
diff --git a/actionview/lib/action_view/path_set.rb b/actionview/lib/action_view/path_set.rb
index 7a88f6bc50..f68d2a77ed 100644
--- a/actionview/lib/action_view/path_set.rb
+++ b/actionview/lib/action_view/path_set.rb
@@ -46,15 +46,12 @@ 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)
-      prefixes = [prefixes] if String === prefixes
-      prefixes.each do |prefix|
-        paths.each do |resolver|
-          templates = resolver.find_all(path, prefix, *args)
-          return templates unless templates.empty?
-        end
-      end
-      []
+      _find_all path, prefixes, args, false
     end
 
     def exists?(path, prefixes, *args)
@@ -72,6 +69,21 @@ module ActionView #:nodoc:
 
     private
 
+    def _find_all(path, prefixes, args, outside_app)
+      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
+          return templates unless templates.empty?
+        end
+      end
+      []
+    end
+
     def typecast(paths)
       paths.map do |path|
         case path
diff --git a/actionview/lib/action_view/renderer/abstract_renderer.rb b/actionview/lib/action_view/renderer/abstract_renderer.rb
index 1f122f9bc6..aa77a77acf 100644
--- a/actionview/lib/action_view/renderer/abstract_renderer.rb
+++ b/actionview/lib/action_view/renderer/abstract_renderer.rb
@@ -15,7 +15,7 @@ module ActionView
   # that new object is called in turn. This abstracts the setup and rendering
   # into a separate classes for partials and templates.
   class AbstractRenderer #:nodoc:
-    delegate :find_template, :template_exists?, :with_fallbacks, :with_layout_format, :formats, :to => :@lookup_context
+    delegate :find_template, :find_file, :template_exists?, :with_fallbacks, :with_layout_format, :formats, :to => :@lookup_context
 
     def initialize(lookup_context)
       @lookup_context = lookup_context
diff --git a/actionview/lib/action_view/renderer/template_renderer.rb b/actionview/lib/action_view/renderer/template_renderer.rb
index 75217e1630..9d15bbfca7 100644
--- a/actionview/lib/action_view/renderer/template_renderer.rb
+++ b/actionview/lib/action_view/renderer/template_renderer.rb
@@ -29,7 +29,7 @@ module ActionView
       elsif options.key?(:html)
         Template::HTML.new(options[:html], formats.first)
       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/actionview/lib/action_view/template/resolver.rb b/actionview/lib/action_view/template/resolver.rb
index 7859c58b43..c92f19689c 100644
--- a/actionview/lib/action_view/template/resolver.rb
+++ b/actionview/lib/action_view/template/resolver.rb
@@ -126,6 +126,12 @@ module ActionView
       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
+
     def find_all_with_query(query) # :nodoc:
       @cache.cache_query(query) { find_template_paths(File.join(@path, query)) }
     end
@@ -187,15 +193,16 @@ module ActionView
 
     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)
+      template_paths = reject_files_external_to_app(template_paths) unless outside_app_allowed
 
       template_paths.map do |template|
         handler, format, variant = extract_handler_and_format_and_variant(template, formats)
@@ -210,6 +217,10 @@ module ActionView
       end
     end
 
+    def reject_files_external_to_app(files)
+      files.reject { |filename| !inside_path?(@path, filename) }
+    end
+
     def find_template_paths(query)
       Dir[query].reject do |filename|
         File.directory?(filename) ||
@@ -218,6 +229,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/actionview/lib/action_view/testing/resolvers.rb b/actionview/lib/action_view/testing/resolvers.rb
index 63a60542d4..2664aca991 100644
--- a/actionview/lib/action_view/testing/resolvers.rb
+++ b/actionview/lib/action_view/testing/resolvers.rb
@@ -19,7 +19,7 @@ module ActionView #:nodoc:
 
   private
 
-    def query(path, exts, formats)
+    def query(path, exts, formats, _)
       query = ""
       EXTENSIONS.each_key do |ext|
         query << '(' << exts[ext].map {|e| e && Regexp.escape(".#{e}") }.join('|') << '|)'
@@ -44,7 +44,7 @@ module ActionView #:nodoc:
   end
 
   class NullResolver < PathResolver
-    def query(path, exts, formats)
+    def query(path, exts, formats, _)
       handler, format, variant = extract_handler_and_format_and_variant(path, formats)
       [ActionView::Template.new("Template generated by Null Resolver", path.virtual, handler, :virtual_path => path.virtual, :format => format, :variant => variant)]
     end
diff --git a/actionview/test/template/render_test.rb b/actionview/test/template/render_test.rb
index 994fd44c52..df27b52197 100644
--- a/actionview/test/template/render_test.rb
+++ b/actionview/test/template/render_test.rb
@@ -148,6 +148,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