[prev in list] [next in list] [prev in thread] [next in thread] 

List:       oss-security
Subject:    [oss-security] [CVE-2016-0752] Possible Information Leak Vulnerability in Action View
From:       Aaron Patterson <tenderlove () ruby-lang ! org>
Date:       2016-01-25 19:36:41
Message-ID: 20160125193641.GF14069 () TC ! local
[Download RAW message or body]

[Attachment #2 (multipart/mixed)]


Possible Information Leak Vulnerability in Action View

There is a possible directory traversal and information leak vulnerability in
Action View. This vulnerability has been assigned the CVE identifier
CVE-2016-0752.

Versions Affected:  All.
Not affected:       None.
Fixed Versions:     5.0.0.beta1.1, 4.2.5.1, 4.1.14.1, 3.2.22.1

Impact
------
Applications that pass unverified user input to the `render` method in a
controller may be vulnerable to an information leak vulnerability.

Impacted code will look something like this:

```ruby
def index
  render params[:id]
end
```

Carefully crafted requests can cause the above code to render files from
unexpected places like outside the application's view directory, and can
possibly escalate this to a remote code execution attack.

All users running an affected release should either upgrade or use one of the
workarounds immediately.

Releases
--------
The FIXED releases are available at the normal locations.

Workarounds
-----------
A workaround to this issue is to not pass arbitrary user input to the `render`
method.  Instead, verify that data before passing it to the `render` method.

For example, change this:

```ruby
def index
  render params[:id]
end
```

To this:

```ruby
def index
  render verify_template(params[:id])
end

private
def verify_template(name)
  # add verification logic particular to your application here
end
```

Patches
-------
To aid users who aren't able to upgrade immediately we have provided patches for
the two supported release series. They are in git-am format and consist of a
single changeset.

* 3-2-render_data_leak.patch - Patch for 3.2 series
* 4-1-render_data_leak.patch - Patch for 4.1 series
* 4-2-render_data_leak.patch - Patch for 4.2 series
* 5-0-render_data_leak.patch - Patch for 5.0 series

Please note that only the 4.1.x and 4.2.x series are supported at present. Users
of earlier unsupported releases are advised to upgrade as soon as possible as we
cannot guarantee the continued availability of security fixes for unsupported
releases.

Credits
-------
Thanks John Poulin for reporting this!

-- 
Aaron Patterson
http://tenderlovemaking.com/

["3-2-render_data_leak.patch" (text/plain)]

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


["4-1-render_data_leak.patch" (text/plain)]

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


["4-2-render_data_leak.patch" (text/plain)]

From cb7ec87047c5093ad66fc62cbe514a4fc7e7b7f3 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 4aaf7ec..a2d87a8 100644
--- a/actionpack/test/controller/render_test.rb
+++ b/actionpack/test/controller/render_test.rb
@@ -58,6 +58,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'
@@ -273,6 +283,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 ea687d9..cb34391 100644
--- a/actionview/lib/action_view/lookup_context.rb
+++ b/actionview/lib/action_view/lookup_context.rb
@@ -122,6 +122,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 1f122f9..aa77a77 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 cd21d7a..73b6077 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 b65507f..d84cb1f 100644
--- a/actionview/lib/action_view/template/resolver.rb
+++ b/actionview/lib/action_view/template/resolver.rb
@@ -113,7 +113,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
 
@@ -174,15 +180,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)
@@ -197,6 +204,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|
@@ -217,6 +228,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 5cccafe..1a505f7 100644
--- a/actionview/test/template/render_test.rb
+++ b/actionview/test/template/render_test.rb
@@ -141,6 +141,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


["5-0-render_data_leak.patch" (text/plain)]

From 0e83ad20ac40c5d2cae37e910a8a0683a4945faa 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


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             | 28 +++++++++++++------
 .../lib/action_view/renderer/abstract_renderer.rb  |  2 +-
 .../lib/action_view/renderer/template_renderer.rb  |  2 +-
 actionview/lib/action_view/template/resolver.rb    | 23 +++++++++++++---
 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 a73f188..63fd76d 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 256ebf6..bbc216b 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 63a3c4e..6a76d80 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 7a88f6b..f68d2a7 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 1f122f9..aa77a77 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 75217e1..9d15bbf 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 6ddd2b6..8a675cd 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 63a6054..2664aca 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 b63c315..333e0cc 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
-- 
2.2.1


[Attachment #9 (application/pgp-signature)]

[prev in list] [next in list] [prev in thread] [next in thread]