Attachment 1119446 Details for Bug 1301963 – 4.1 patch

[patch] 4.1 patch

4-1-render_data_leak.patch (text/plain), 11.33 KB, created by Ján Rusnačko on 2016-01-29 15:02:41 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Ján Rusnačko

Created: 2016-01-29 15:02:41 UTC

Size: 11.33 KB

patch

obsolete

>From 5c656a271a890cca4b3d438cc1fc76ff98011cbe Mon Sep 17 00:00:00 2001
>From: Aaron Patterson <aaron.patterson@gmail.com>
>Date: Wed, 20 Jan 2016 10:39:19 -0800
>Subject: [PATCH] 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
>---
> actionpack/lib/abstract_controller/rendering.rb    |  8 +++++-
> actionpack/test/controller/render_test.rb          | 31 ++++++++++++++++++++++
> actionview/lib/action_view/lookup_context.rb       |  4 +++
> actionview/lib/action_view/path_set.rb             | 26 +++++++++++++-----
> .../lib/action_view/renderer/abstract_renderer.rb  |  2 +-
> .../lib/action_view/renderer/template_renderer.rb  |  2 +-
> actionview/lib/action_view/template/resolver.rb    | 25 ++++++++++++++---
> actionview/lib/action_view/testing/resolvers.rb    |  4 +--
> actionview/test/template/render_test.rb            |  7 +++++
> 9 files changed, 93 insertions(+), 16 deletions(-)
>
>diff --git a/actionpack/lib/abstract_controller/rendering.rb b/actionpack/lib/abstract_controller/rendering.rb
>index 9d10140..e80d97f 100644
>--- a/actionpack/lib/abstract_controller/rendering.rb
>+++ b/actionpack/lib/abstract_controller/rendering.rb
>@@ -77,7 +77,13 @@ module AbstractController
>     # render "foo/bar" to render :file => "foo/bar".
>     # :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 26806fb..17a019e 100644
>--- a/actionpack/test/controller/render_test.rb
>+++ b/actionpack/test/controller/render_test.rb
>@@ -52,6 +52,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
>+
>   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'
>@@ -251,6 +261,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, { 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 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 855fed0..93ef701 100644
>--- a/actionview/lib/action_view/lookup_context.rb
>+++ b/actionview/lib/action_view/lookup_context.rb
>@@ -125,6 +125,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 91ee2ea..8d21913 100644
>--- a/actionview/lib/action_view/path_set.rb
>+++ b/actionview/lib/action_view/path_set.rb
>@@ -46,23 +46,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/actionview/lib/action_view/renderer/abstract_renderer.rb b/actionview/lib/action_view/renderer/abstract_renderer.rb
>index 73c19a0..8457008 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 be17097..66b611d 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
>       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 f1bb47a..8d8a37e 100644
>--- a/actionview/lib/action_view/template/resolver.rb
>+++ b/actionview/lib/action_view/template/resolver.rb
>@@ -112,7 +112,13 @@ module ActionView
>     # Normalizes the arguments and passes it on to find_templates.
>     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
> 
>@@ -173,15 +179,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 { |template|
>         handler, format, variant = extract_handler_and_format_and_variant(template, formats)
>@@ -196,6 +203,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|
>@@ -216,6 +227,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 dfb7d46..e88f425 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, handler, :virtual_path => path, :format => format, :variant => variant)]
>     end
>diff --git a/actionview/test/template/render_test.rb b/actionview/test/template/render_test.rb
>index 1316f85..caf6d13 100644
>--- a/actionview/test/template/render_test.rb
>+++ b/actionview/test/template/render_test.rb
>@@ -142,6 +142,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
>-- 
>2.2.1
>

Actions: View | Diff

Attachments on bug 1301963: 1119445 | 1119446 | 1119447 | 1119449