Attachment 675070 Details for Bug 892870 – actionpack-CVE-2012-0156-3-0-null_array_param.patch

[patch] actionpack-CVE-2012-0156-3-0-null_array_param.patch

actionpack-CVE-2012-0156-3-0-null_array_param.patch (text/plain), 7.96 KB, created by Kurt Seifried on 2013-01-08 21:19:57 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kurt Seifried

Created: 2013-01-08 21:19:57 UTC

Size: 7.96 KB

patch

obsolete

>From a494824efc3212232f6b6328e7a1951e9cf17105 Mon Sep 17 00:00:00 2001
>From: Jeremy Kemper <jeremy@bitsweat.net>
>Date: Sat, 5 Jan 2013 17:46:26 -0700
>Subject: [PATCH] CVE-2013-0156: Safe XML params parsing. Doesn't allow
> symbols or yaml.
>
>---
> actionpack/test/controller/webservice_test.rb      |   13 ++++++++
> activesupport/CHANGELOG                            |    9 ++++++
> .../active_support/core_ext/hash/conversions.rb    |   32 +++++++++++++++-----
> activesupport/test/core_ext/hash_ext_test.rb       |   28 +++++++++++++----
> 4 files changed, 69 insertions(+), 13 deletions(-)
>
>diff --git a/actionpack/test/controller/webservice_test.rb b/actionpack/test/controller/webservice_test.rb
>index 9a9e7fd..8a44551 100644
>--- a/actionpack/test/controller/webservice_test.rb
>+++ b/actionpack/test/controller/webservice_test.rb
>@@ -117,6 +117,19 @@ class WebServiceTest < ActionController::IntegrationTest
>     end
>   end
> 
>+  def test_post_xml_using_a_disallowed_type_attribute
>+    $stderr = StringIO.new
>+    with_test_route_set do
>+      post '/', '<foo type="symbol">value</foo>', 'CONTENT_TYPE' => 'application/xml'
>+      assert_response 500
>+
>+      post '/', '<foo type="yaml">value</foo>', 'CONTENT_TYPE' => 'application/xml'
>+      assert_response 500
>+    end
>+  ensure
>+    $stderr = STDERR
>+  end
>+
>   def test_register_and_use_yaml
>     with_test_route_set do
>       with_params_parsers Mime::YAML => Proc.new { |d| YAML.load(d) } do
>diff --git a/activesupport/CHANGELOG b/activesupport/CHANGELOG
>index f156bf3..56259b9 100644
>--- a/activesupport/CHANGELOG
>+++ b/activesupport/CHANGELOG
>@@ -1,3 +1,12 @@
>+## Rails 3.0.19 (Jan 8, 2012) ##
>+
>+*   Hash.from_xml raises when it encounters type="symbol" or type="yaml".
>+    Use Hash.from_trusted_xml to parse this XML.
>+
>+    CVE-2013-0156
>+
>+    *Jeremy Kemper*
>+
> ## Rails 3.0.18
> 
> ## Rails 3.0.17 (Aug 9, 2012)
>diff --git a/activesupport/lib/active_support/core_ext/hash/conversions.rb b/activesupport/lib/active_support/core_ext/hash/conversions.rb
>index 465a635..e0856f6 100644
>--- a/activesupport/lib/active_support/core_ext/hash/conversions.rb
>+++ b/activesupport/lib/active_support/core_ext/hash/conversions.rb
>@@ -73,15 +73,33 @@ class Hash
>     end
>   end
> 
>+  class DisallowedType < StandardError #:nodoc:
>+    def initialize(type)
>+      super "Disallowed type attribute: #{type.inspect}"
>+    end
>+  end
>+
>+  DISALLOWED_XML_TYPES = %w(symbol yaml)
>+
>   class << self
>-    def from_xml(xml)
>-      typecast_xml_value(unrename_keys(ActiveSupport::XmlMini.parse(xml)))
>+    def from_xml(xml, disallowed_types = nil)
>+      typecast_xml_value(unrename_keys(ActiveSupport::XmlMini.parse(xml)), disallowed_types)
>+    end
>+
>+    def from_trusted_xml(xml)
>+      from_xml xml, []
>     end
> 
>     private
>-      def typecast_xml_value(value)
>+      def typecast_xml_value(value, disallowed_types = nil)
>+        disallowed_types ||= DISALLOWED_XML_TYPES
>+
>         case value.class.to_s
>           when 'Hash'
>+            if value.include?('type') && !value['type'].is_a?(Hash) && disallowed_types.include?(value['type'])
>+              raise DisallowedType, value['type']
>+            end
>+
>             if value['type'] == 'array'
>               _, entries = Array.wrap(value.detect { |k,v| k != 'type' })
>               if entries.nil? || (c = value['__content__'] && c.blank?)
>@@ -89,9 +107,9 @@ class Hash
>               else
>                 case entries.class.to_s   # something weird with classes not matching here.  maybe singleton methods breaking is_a?
>                 when "Array"
>-                  entries.collect { |v| typecast_xml_value(v) }
>+                  entries.collect { |v| typecast_xml_value(v, disallowed_types) }
>                 when "Hash"
>-                  [typecast_xml_value(entries)]
>+                  [typecast_xml_value(entries, disallowed_types)]
>                 else
>                   raise "can't typecast #{entries.inspect}"
>                 end
>@@ -116,7 +134,7 @@ class Hash
>               nil
>             else
>               xml_value = value.inject({}) do |h,(k,v)|
>-                h[k] = typecast_xml_value(v)
>+                h[k] = typecast_xml_value(v, disallowed_types)
>                 h
>               end
> 
>@@ -125,7 +143,7 @@ class Hash
>               xml_value["file"].is_a?(StringIO) ? xml_value["file"] : xml_value
>             end
>           when 'Array'
>-            value.map! { |i| typecast_xml_value(i) }
>+            value.map! { |i| typecast_xml_value(i, disallowed_types) }
>             value.length > 1 ? value : value.first
>           when 'String'
>             value
>diff --git a/activesupport/test/core_ext/hash_ext_test.rb b/activesupport/test/core_ext/hash_ext_test.rb
>index e9aa57c..ff1408d 100644
>--- a/activesupport/test/core_ext/hash_ext_test.rb
>+++ b/activesupport/test/core_ext/hash_ext_test.rb
>@@ -640,12 +640,10 @@ class HashToXmlTest < Test::Unit::TestCase
>         <replies-close-in type="integer">2592000000</replies-close-in>
>         <written-on type="date">2003-07-16</written-on>
>         <viewed-at type="datetime">2003-07-16T09:28:00+0000</viewed-at>
>-        <content type="yaml">--- \n1: should be an integer\n:message: Have a nice day\narray: \n- should-have-dashes: true\n  should_have_underscores: true\n</content>
>         <author-email-address>david@loudthinking.com</author-email-address>
>         <parent-id></parent-id>
>         <ad-revenue type="decimal">1.5</ad-revenue>
>         <optimum-viewing-angle type="float">135</optimum-viewing-angle>
>-        <resident type="symbol">yes</resident>
>       </topic>
>     EOT
> 
>@@ -658,12 +656,10 @@ class HashToXmlTest < Test::Unit::TestCase
>       :replies_close_in => 2592000000,
>       :written_on => Date.new(2003, 7, 16),
>       :viewed_at => Time.utc(2003, 7, 16, 9, 28),
>-      :content => { :message => "Have a nice day", 1 => "should be an integer", "array" => [{ "should-have-dashes" => true, "should_have_underscores" => true }] },
>       :author_email_address => "david@loudthinking.com",
>       :parent_id => nil,
>       :ad_revenue => BigDecimal("1.50"),
>       :optimum_viewing_angle => 135.0,
>-      :resident => :yes
>     }.stringify_keys
> 
>     assert_equal expected_topic_hash, Hash.from_xml(topic_xml)["topic"]
>@@ -677,7 +673,6 @@ class HashToXmlTest < Test::Unit::TestCase
>         <approved type="boolean"></approved>
>         <written-on type="date"></written-on>
>         <viewed-at type="datetime"></viewed-at>
>-        <content type="yaml"></content>
>         <parent-id></parent-id>
>       </topic>
>     EOT
>@@ -688,7 +683,6 @@ class HashToXmlTest < Test::Unit::TestCase
>       :approved   => nil,
>       :written_on => nil,
>       :viewed_at  => nil,
>-      :content    => nil,
>       :parent_id  => nil
>     }.stringify_keys
> 
>@@ -915,6 +909,28 @@ class HashToXmlTest < Test::Unit::TestCase
>     assert_equal expected_product_hash, Hash.from_xml(product_xml)["product"]
>   end
> 
>+  def test_from_xml_raises_on_disallowed_type_attributes
>+    assert_raise Hash::DisallowedType do
>+      Hash.from_xml '<product><name type="foo">value</name></product>', %w(foo)
>+    end
>+  end
>+
>+  def test_from_xml_disallows_symbol_and_yaml_types_by_default
>+    assert_raise Hash::DisallowedType do
>+      Hash.from_xml '<product><name type="symbol">value</name></product>'
>+    end
>+
>+    assert_raise Hash::DisallowedType do
>+      Hash.from_xml '<product><name type="yaml">value</name></product>'
>+    end
>+  end
>+
>+  def test_from_trusted_xml_allows_symbol_and_yaml_types
>+    expected = { 'product' => { 'name' => :value }}
>+    assert_equal expected, Hash.from_trusted_xml('<product><name type="symbol">value</name></product>')
>+    assert_equal expected, Hash.from_trusted_xml('<product><name type="yaml">:value</name></product>')
>+  end
>+
>   def test_should_use_default_value_for_unknown_key
>     hash_wia = HashWithIndifferentAccess.new(3)
>     assert_equal 3, hash_wia[:new_key]
>-- 
>1.7.10.2 (Apple Git-33)
>

Actions: View | Diff

Attachments on bug 892870: 674509 | 674510 | 674511 | 674512 | 675064 | 675066 | 675067 | 675068 | 675069 | 675070 | 675071 | 675072 | 675077 | 675078 | 675079 | 675080