[prev in list] [next in list] [prev in thread] [next in thread]
List: oss-security
Subject: [oss-security] [CVE-2016-2098] Possible remote code execution vulnerability in Action Pack
From: Rafael Mendonça França <rafaelmfranca () gmail ! com>
Date: 2016-02-29 19:31:17
Message-ID: CAC9YFzf4JBQW5oSk49xAqWv9EwqVU+i_5vcAj5A8gkWw2Tu9Cw () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
There is a possible remote code execution vulnerability in Action Pack.
This vulnerability has been assigned the CVE identifier CVE-2016-2098.
Versions Affected: 3.2.x, 4.0.x, 4.1.x, 4.2.x
Not affected: 5.0+
Fixed Versions: 3.2.22.2, 4.1.14.2, 4.2.5.2
Impact
------
Applications that pass unverified user input to the `render` method in a
controller or a view may be vulnerable to a code injection.
Impacted code will look like this:
```ruby
class TestController < ApplicationController
def show
render params[:id]
end
end
```
An attacker could use the request parameters to coerce the above example
to execute arbitrary ruby code.
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 show
render params[:id]
end
```
To this:
```ruby
def show
render verify_id(params[:id])
end
private
def verify_id(id)
# add verification logic particular to your application here
end
```
Patches
-------
To aid users who aren't able to upgrade immediately we have provided a patch for
it. It is in git-am format and consist of a single changeset.
* 3-2-secure_inline_with_params.patch - Patch for 3.2 series
* 4-1-secure_inline_with_params.patch - Patch for 4.1 series
* 4-2-secure_inline_with_params.patch - Patch for 4.2 series
Credits
-------
Thanks to both Tobias Kraze from makandra and joernchen of Phenoelit
for reporting this!
[Attachment #5 (text/html)]
<div dir="ltr"><pre style="color:rgb(0,0,0);line-height:normal">There is a possible remote code \
execution vulnerability in Action Pack. This vulnerability has been assigned the CVE identifier \
CVE-2016-2098.
Versions Affected: 3.2.x, 4.0.x, 4.1.x, 4.2.x
Not affected: 5.0+
Fixed Versions: 3.2.22.2, 4.1.14.2, 4.2.5.2
Impact
------
Applications that pass unverified user input to the `render` method in a
controller or a view may be vulnerable to a code injection.
Impacted code will look like this:
```ruby
class TestController < ApplicationController
def show
render params[:id]
end
end
```
An attacker could use the request parameters to coerce the above example
to execute arbitrary ruby code.
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 show
render params[:id]
end
```
To this:
```ruby
def show
render verify_id(params[:id])
end
private
def verify_id(id)
# add verification logic particular to your application here
end
```
Patches
-------
To aid users who aren't able to upgrade immediately we have provided a patch for
it. It is in git-am format and consist of a single changeset.
* 3-2-secure_inline_with_params.patch - Patch for 3.2 series
* 4-1-secure_inline_with_params.patch - Patch for 4.1 series
* 4-2-secure_inline_with_params.patch - Patch for 4.2 series
Credits
-------
Thanks to both Tobias Kraze from makandra and joernchen of Phenoelit for reporting \
this!</pre></div>
--001a11c2d760ec67a5052cedb1cb--
["4-2-secure_inline_with_params.patch" (application/octet-stream)]
From f8e2fe8810d67adfcef8acd95b0e51a31de16acd Mon Sep 17 00:00:00 2001
From: Arthur Neves <arthurnn@gmail.com>
Date: Wed, 24 Feb 2016 20:29:10 -0500
Subject: [PATCH] Don't allow render(params) on views.
If `render(params)` is called in a view it should be protected the same
way it is in the controllers. We should raise an error if thats happens.
Fix CVE-2016-2098.
---
actionpack/test/controller/render_test.rb | 24 +++++++++++++++++++++++-
actionview/lib/action_view/renderer/renderer.rb | 4 ++++
actionview/test/template/render_test.rb | 19 +++++++++++++++++++
3 files changed, 46 insertions(+), 1 deletion(-)
diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb
index a2d87a8..d607405 100644
--- a/actionpack/test/controller/render_test.rb
+++ b/actionpack/test/controller/render_test.rb
@@ -280,6 +280,16 @@ class MetalTestController < ActionController::Metal
end
end
+class MetalWithoutAVTestController < ActionController::Metal
+ include AbstractController::Rendering
+ include ActionController::Rendering
+ include ActionController::StrongParameters
+
+ def dynamic_params_render
+ render params
+ end
+end
+
class ExpiresInRenderTest < ActionController::TestCase
tests TestController
@@ -299,9 +309,10 @@ class ExpiresInRenderTest < ActionController::TestCase
end
def test_dynamic_render_file_hash
- assert_raises ArgumentError do
+ e = assert_raises ArgumentError do
get :dynamic_render, { id: { file: '../\\../test/abstract_unit.rb' } }
end
+ assert_equal "render parameters are not permitted", e.message
end
def test_expires_in_header
@@ -500,6 +511,17 @@ class MetalRenderTest < ActionController::TestCase
end
end
+class MetalRenderWithoutAVTest < ActionController::TestCase
+ tests MetalWithoutAVTestController
+
+ def test_dynamic_params_render
+ e = assert_raises ArgumentError do
+ get :dynamic_params_render, { inline: '<%= RUBY_VERSION %>' }
+ end
+ assert_equal "render parameters are not permitted", e.message
+ end
+end
+
class HeadRenderTest < ActionController::TestCase
tests TestController
diff --git a/actionview/lib/action_view/renderer/renderer.rb b/actionview/lib/action_view/renderer/renderer.rb
index 964b183..5ba7b2b 100644
--- a/actionview/lib/action_view/renderer/renderer.rb
+++ b/actionview/lib/action_view/renderer/renderer.rb
@@ -17,6 +17,10 @@ module ActionView
# Main render entry point shared by AV and AC.
def render(context, options)
+ if options.respond_to?(:permitted?) && !options.permitted?
+ raise ArgumentError, "render parameters are not permitted"
+ end
+
if options.key?(:partial)
render_partial(context, options)
else
diff --git a/actionview/test/template/render_test.rb b/actionview/test/template/render_test.rb
index 6b65bfb..b0af6ea 100644
--- a/actionview/test/template/render_test.rb
+++ b/actionview/test/template/render_test.rb
@@ -148,6 +148,25 @@ module RenderTestCases
end
end
+ def test_render_with_strong_parameters
+ params = { :inline => '<%= RUBY_VERSION %>' }
+ def params.permitted?
+ false
+ end
+ e = assert_raises ArgumentError do
+ @view.render(params)
+ end
+ assert_equal "render parameters are not permitted", e.message
+ end
+
+ def test_render_with_permitted_strong_parameters
+ params = { inline: "<%= 'hello' %>" }
+ def params.permitted?
+ true
+ end
+ assert_equal 'hello', @view.render(params)
+ end
+
def test_render_partial
assert_equal "only partial", @view.render(:partial => "test/partial_only")
end
--
2.5.4 (Apple Git-61)
["3-2-secure_inline_with_params.patch" (application/octet-stream)]
From 1f2eb59c9858b35564cf8e8508abdf63321a8d61 Mon Sep 17 00:00:00 2001
From: Arthur Neves <arthurnn@gmail.com>
Date: Tue, 2 Feb 2016 12:34:11 -0500
Subject: [PATCH 2/2] Don't allow render(params) in view/controller
`render(params)` is dangerous and could be a vector for attackers.
Don't allow calls to render passing params on views or controllers.
On a controller or view, we should not allow something like `render
params[:id]` or `render params`.
That could be problematic, because an attacker could pass input that
could lead to a remote code execution attack.
This patch is also compatible when using strong parameters.
CVE-2016-2098
---
actionpack/lib/action_view/renderer/renderer.rb | 5 +++
actionpack/test/controller/render_test.rb | 53 ++++++++++++++++++++++---
actionpack/test/template/render_test.rb | 27 +++++++++++++
3 files changed, 79 insertions(+), 6 deletions(-)
diff --git a/actionpack/lib/action_view/renderer/renderer.rb b/actionpack/lib/action_view/renderer/renderer.rb
index bf1b5a7..0f35989 100644
--- a/actionpack/lib/action_view/renderer/renderer.rb
+++ b/actionpack/lib/action_view/renderer/renderer.rb
@@ -11,6 +11,11 @@ module ActionView
# Main render entry point shared by AV and AC.
def render(context, options)
+ if (options.is_a?(HashWithIndifferentAccess) && !options.respond_to?(:permitted?)) ||
+ (options.respond_to?(:permitted?) && !options.permitted?)
+ raise ArgumentError, "render parameters are not permitted"
+ end
+
if options.key?(:partial)
render_partial(context, options)
else
diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb
index bc42c66..de80b78 100644
--- a/actionpack/test/controller/render_test.rb
+++ b/actionpack/test/controller/render_test.rb
@@ -70,6 +70,10 @@ class TestController < ActionController::Base
render :file => file
end
+ def dynamic_inline_render
+ render :inline => "<%= render(params) %>"
+ 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'
@@ -245,12 +249,6 @@ class TestController < ActionController::Base
render :inline => "<%= action_name %>"
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
@@ -742,6 +740,15 @@ class MetalTestController < ActionController::Metal
end
end
+class MetalWithoutAVTestController < ActionController::Metal
+ include AbstractController::Rendering
+ include ActionController::Rendering
+
+ def dynamic_params_render
+ render params
+ end
+end
+
class RenderTest < ActionController::TestCase
tests TestController
@@ -783,6 +790,29 @@ class RenderTest < ActionController::TestCase
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_dynamic_inline
+ assert_raises ArgumentError do
+ get :dynamic_render, { :id => { :inline => '<%= RUBY_VERSION %>' } }
+ end
+ end
+
+ def test_dynamic_render_on_view
+ file = Tempfile.new('_name')
+ file.write "secrets!"
+ file.flush
+
+ e = assert_raises ActionView::Template::Error do
+ get :dynamic_inline_render, { :file => file.path }
+ end
+ assert_equal "render parameters are not permitted", e.message
+ end
+
# :ported:
def test_simple_show
get :hello_world
@@ -1657,3 +1687,14 @@ class MetalRenderTest < ActionController::TestCase
assert_equal "NilClass", @response.body
end
end
+
+class MetalRenderWithoutAVTest < ActionController::TestCase
+ tests MetalWithoutAVTestController
+
+ def test_dynamic_params_render
+ e = assert_raises ArgumentError do
+ get :dynamic_params_render, { inline: '<%= RUBY_VERSION %>' }
+ end
+ assert_equal "render parameters are not permitted", e.message
+ end
+end
diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb
index f207ad7..2d04665 100644
--- a/actionpack/test/template/render_test.rb
+++ b/actionpack/test/template/render_test.rb
@@ -133,6 +133,33 @@ module RenderTestCases
end
end
+ def test_render_with_params
+ params = { :inline => '<%= RUBY_VERSION %>' }.with_indifferent_access
+ assert_raises ArgumentError do
+ @view.render(params)
+ end
+ end
+
+ def test_render_with_strong_parameters
+ # compatibility with Strong Parameters gem
+ params = Class.new(HashWithIndifferentAccess).new
+ params[:inline] = '<%= RUBY_VERSION %>'
+ e = assert_raises ArgumentError do
+ @view.render(params)
+ end
+ assert_equal "render parameters are not permitted", e.message
+ end
+
+ def test_render_with_permitted_strong_parameters
+ # compatibility with Strong Parameters gem
+ params = Class.new(HashWithIndifferentAccess).new
+ params[:inline] = "<%= 'hello' %>"
+ def params.permitted?
+ true
+ end
+ assert_equal 'hello', @view.render(params)
+ end
+
def test_render_partial
assert_equal "only partial", @view.render(:partial => "test/partial_only")
end
--
2.5.4 (Apple Git-61)
["4-1-secure_inline_with_params.patch" (application/octet-stream)]
From 1b84d905801125fcca0c8f43bf6af7d7872ac87e Mon Sep 17 00:00:00 2001
From: Arthur Neves <arthurnn@gmail.com>
Date: Wed, 24 Feb 2016 20:29:10 -0500
Subject: [PATCH 2/2] Don't allow render(params) on views.
If `render(params)` is called in a view it should be protected the same
way it is in the controllers. We should raise an error if thats happens.
Fix CVE-2016-2098.
---
actionpack/test/controller/render_test.rb | 24 +++++++++++++++++++++++-
actionview/lib/action_view/renderer/renderer.rb | 4 ++++
actionview/test/template/render_test.rb | 19 +++++++++++++++++++
3 files changed, 46 insertions(+), 1 deletion(-)
diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb
index 0fcbb86..7bdf65c 100644
--- a/actionpack/test/controller/render_test.rb
+++ b/actionpack/test/controller/render_test.rb
@@ -258,6 +258,16 @@ class MetalTestController < ActionController::Metal
end
end
+class MetalWithoutAVTestController < ActionController::Metal
+ include AbstractController::Rendering
+ include ActionController::Rendering
+ include ActionController::StrongParameters
+
+ def dynamic_params_render
+ render params
+ end
+end
+
class ExpiresInRenderTest < ActionController::TestCase
tests TestController
@@ -294,9 +304,10 @@ class ExpiresInRenderTest < ActionController::TestCase
end
def test_dynamic_render_file_hash
- assert_raises ArgumentError do
+ e = assert_raises ArgumentError do
get :dynamic_render, { id: { file: '../\\../test/abstract_unit.rb' } }
end
+ assert_equal "render parameters are not permitted", e.message
end
def test_expires_in_header
@@ -473,6 +484,17 @@ class MetalRenderTest < ActionController::TestCase
end
end
+class MetalRenderWithoutAVTest < ActionController::TestCase
+ tests MetalWithoutAVTestController
+
+ def test_dynamic_params_render
+ e = assert_raises ArgumentError do
+ get :dynamic_params_render, { inline: '<%= RUBY_VERSION %>' }
+ end
+ assert_equal "render parameters are not permitted", e.message
+ end
+end
+
class HeadRenderTest < ActionController::TestCase
tests TestController
diff --git a/actionview/lib/action_view/renderer/renderer.rb b/actionview/lib/action_view/renderer/renderer.rb
index 964b183..5ba7b2b 100644
--- a/actionview/lib/action_view/renderer/renderer.rb
+++ b/actionview/lib/action_view/renderer/renderer.rb
@@ -17,6 +17,10 @@ module ActionView
# Main render entry point shared by AV and AC.
def render(context, options)
+ if options.respond_to?(:permitted?) && !options.permitted?
+ raise ArgumentError, "render parameters are not permitted"
+ end
+
if options.key?(:partial)
render_partial(context, options)
else
diff --git a/actionview/test/template/render_test.rb b/actionview/test/template/render_test.rb
index caf6d13..b3de94f 100644
--- a/actionview/test/template/render_test.rb
+++ b/actionview/test/template/render_test.rb
@@ -149,6 +149,25 @@ module RenderTestCases
end
end
+ def test_render_with_strong_parameters
+ params = { :inline => '<%= RUBY_VERSION %>' }
+ def params.permitted?
+ false
+ end
+ e = assert_raises ArgumentError do
+ @view.render(params)
+ end
+ assert_equal "render parameters are not permitted", e.message
+ end
+
+ def test_render_with_permitted_strong_parameters
+ params = { inline: "<%= 'hello' %>" }
+ def params.permitted?
+ true
+ end
+ assert_equal 'hello', @view.render(params)
+ end
+
def test_render_partial
assert_equal "only partial", @view.render(:partial => "test/partial_only")
end
--
2.5.4 (Apple Git-61)
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic