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

actionpack/lib/abstract_controller/rendering.rb

@@ -147,12 +147,11 @@ def _normalize_args(action=nil, options={})
         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
 

actionpack/lib/action_view/lookup_context.rb

@@ -127,6 +127,10 @@ def find_all(name, prefixes = [], partial = false, keys = [], options = {})
         @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

actionpack/lib/action_view/path_set.rb

@@ -58,23 +58,35 @@ def find(*args)
       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

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)

actionpack/lib/action_view/renderer/template_renderer.rb

@@ -21,11 +21,12 @@ def render(context, options)
     # 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)

actionpack/lib/action_view/template/resolver.rb

@@ -43,7 +43,13 @@ def clear_cache
     # 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 @@ def find_all(name, prefix=nil, partial=false, details={}, key=nil, locals=[])
     # 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 @@ def initialize(pattern=nil)
       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 @@ def eql?(resolver)
   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}}"

actionpack/lib/action_view/testing/resolvers.rb

@@ -19,7 +19,7 @@ def to_s
 
   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 @@ def query(path, exts, formats)
   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
-

actionpack/test/controller/new_base/render_file_test.rb

@@ -13,15 +13,6 @@ def with_instance_variables
       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 @@ def with_locals
       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 @@ class TestBasic < Rack::TestCase
       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 @@ class TestBasic < Rack::TestCase
       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

actionpack/test/controller/render_test.rb

@@ -245,33 +245,6 @@ def accessing_action_name_in_template
     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 @@ def setup
     @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: