Attachment 1119445 Details for Bug 1301963 – 3.2 patch

[patch] 3.2 patch

3-2-render_data_leak.patch (text/plain), 5.95 KB, created by Ján Rusnačko on 2016-01-29 15:02:01 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Ján Rusnačko

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

Size: 5.95 KB

patch

obsolete

>From 6928fa098b0e81c10bba3992c68bac37a149f59d 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/action_view/template/resolver.rb    | 17 ++++++++++++
> .../test/controller/new_base/render_file_test.rb   | 18 ++++++++++---
> actionpack/test/controller/render_test.rb          | 31 ++++++++++++++++++++++
> actionpack/test/template/render_test.rb            |  7 +++++
> 4 files changed, 69 insertions(+), 4 deletions(-)
>
>diff --git a/actionpack/lib/action_view/template/resolver.rb b/actionpack/lib/action_view/template/resolver.rb
>index 47ea8a3..c6db668 100644
>--- a/actionpack/lib/action_view/template/resolver.rb
>+++ b/actionpack/lib/action_view/template/resolver.rb
>@@ -110,6 +110,9 @@ module ActionView
>       super()
>     end
> 
>+    cattr_accessor :allow_external_files, instance_reader: false, instance_writer: false
>+    self.allow_external_files = false
>+
>     private
> 
>     def find_templates(name, prefix, partial, details)
>@@ -122,6 +125,10 @@ module ActionView
> 
>       template_paths = find_template_paths query
> 
>+      unless self.class.allow_external_files
>+        template_paths = reject_files_external_to_app(template_paths)
>+      end
>+
>       template_paths.map { |template|
>         handler, format = extract_handler_and_format(template, formats)
>         contents = File.binread template
>@@ -133,6 +140,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|
>@@ -153,6 +164,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/actionpack/test/controller/new_base/render_file_test.rb b/actionpack/test/controller/new_base/render_file_test.rb
>index a961cbf..c0e23db 100644
>--- a/actionpack/test/controller/new_base/render_file_test.rb
>+++ b/actionpack/test/controller/new_base/render_file_test.rb
>@@ -72,13 +72,23 @@ module RenderFile
>     end
> 
>     test "rendering a relative path" do
>-      get :relative_path
>-      assert_response "The secret is in the sauce\n"
>+      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
>-      get :relative_path_with_dot
>-      assert_response "The secret is in the sauce\n"
>+      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
>diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb
>index 3964540..6210524 100644
>--- a/actionpack/test/controller/render_test.rb
>+++ b/actionpack/test/controller/render_test.rb
>@@ -60,6 +60,16 @@ class TestController < ActionController::Base
>     end
>   end
> 
>+  def dynamic_render
>+    render params[:id] # => String, Hash
>+  end
>+
>+  def dynamic_render_with_file
>+    # This is extremely bad, but should be possible to do.
>+    file = params[:id] # => String, Hash
>+    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'
>@@ -235,6 +245,27 @@ 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
>+    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 accessing_controller_name_in_template
>     render :inline =>  "<%= controller_name %>"
>   end
>diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb
>index 03f3a34..f207ad7 100644
>--- a/actionpack/test/template/render_test.rb
>+++ b/actionpack/test/template/render_test.rb
>@@ -126,6 +126,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