diff options
| -rw-r--r-- | actionpack/test/controller/webservice_test.rb | 13 | ||||
| -rw-r--r-- | activesupport/CHANGELOG.md | 9 | ||||
| -rw-r--r-- | activesupport/lib/active_support/core_ext/hash/conversions.rb | 32 | ||||
| -rw-r--r-- | 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 ae8588cbb0..13b6f4fc39 100644 --- a/actionpack/test/controller/webservice_test.rb +++ b/actionpack/test/controller/webservice_test.rb @@ -118,6 +118,19 @@ class WebServiceTest < ActionDispatch::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.md b/activesupport/CHANGELOG.md index 7faf55bf52..ec198059fb 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,12 @@ +## Rails 3.2.10 (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.2.9 (Nov 12, 2012) ## * Add logger.push_tags and .pop_tags to complement logger.tagged: diff --git a/activesupport/lib/active_support/core_ext/hash/conversions.rb b/activesupport/lib/active_support/core_ext/hash/conversions.rb index 5f07bb4f5a..b820a167a2 100644 --- a/activesupport/lib/active_support/core_ext/hash/conversions.rb +++ b/activesupport/lib/active_support/core_ext/hash/conversions.rb @@ -85,15 +85,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| not v.is_a?(String) }) if entries.nil? || (c = value['__content__'] && c.blank?) @@ -101,9 +119,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 @@ -127,14 +145,14 @@ class Hash elsif value['type'] && value.size == 1 && !value['type'].is_a?(::Hash) nil else - xml_value = Hash[value.map { |k,v| [k, typecast_xml_value(v)] }] + xml_value = Hash[value.map { |k,v| [k, typecast_xml_value(v, disallowed_types)] }] # Turn { :files => { :file => #<StringIO> } into { :files => #<StringIO> } so it is compatible with # how multipart uploaded files from HTML appear 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 b5eb049ad4..c3a59540fb 100644 --- a/activesupport/test/core_ext/hash_ext_test.rb +++ b/activesupport/test/core_ext/hash_ext_test.rb @@ -733,12 +733,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 @@ -751,12 +749,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"] @@ -770,7 +766,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 @@ -781,7 +776,6 @@ class HashToXmlTest < Test::Unit::TestCase :approved => nil, :written_on => nil, :viewed_at => nil, - :content => nil, :parent_id => nil }.stringify_keys @@ -1008,6 +1002,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] |