Package: rails / 2.3.5-1.2+squeeze8

CVE-2013-0156.patch Patch series | download
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
This patch was downloaded from:

http://marc.info/?l=oss-security&m=135767620814533&q=p5

And was modified to remove the activesupport/CHANGELOG hunk that doesn't
apply for us.

From 70adb9613e4a40c5645c99da374639c41012e4fc 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                            |    6 ++++
 .../active_support/core_ext/hash/conversions.rb    |   31 +++++++++++++++-----
 activesupport/test/core_ext/hash_ext_test.rb       |   30 ++++++++++++++-----
 4 files changed, 66 insertions(+), 14 deletions(-)

diff --git a/actionpack/test/controller/webservice_test.rb b/actionpack/test/controller/webservice_test.rb
index e89d6bb..9fea20d 100644
--- a/actionpack/test/controller/webservice_test.rb
+++ b/actionpack/test/controller/webservice_test.rb
@@ -121,6 +121,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
       ActionController::Base.param_parsers[Mime::YAML] = Proc.new { |d| YAML.load(d) }
diff --git a/activesupport/lib/active_support/core_ext/hash/conversions.rb b/activesupport/lib/active_support/core_ext/hash/conversions.rb
index a43763f..d7a8c1e 100644
--- a/activesupport/lib/active_support/core_ext/hash/conversions.rb
+++ b/activesupport/lib/active_support/core_ext/hash/conversions.rb
@@ -26,6 +26,13 @@ module ActiveSupport #:nodoc:
           end
         end
 
+        DISALLOWED_XML_TYPES = %w(symbol yaml)
+        class DisallowedType < StandardError #:nodoc:
+          def initialize(type)
+            super "Disallowed type attribute: #{type.inspect}"
+          end
+        end
+
         XML_TYPE_NAMES = {
           "Symbol"     => "symbol",
           "Fixnum"     => "integer",
@@ -160,14 +167,24 @@ module ActiveSupport #:nodoc:
         end
 
         module ClassMethods
-          def from_xml(xml)
-            typecast_xml_value(unrename_keys(XmlMini.parse(xml)))
+          def from_xml(xml, disallowed_types = nil)
+            typecast_xml_value(unrename_keys(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'
                     child_key, entries = value.detect { |k,v| k != 'type' }   # child_key is throwaway
                     if entries.nil? || (c = value['__content__'] && c.blank?)
@@ -175,9 +192,9 @@ module ActiveSupport #:nodoc:
                     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
@@ -205,7 +222,7 @@ module ActiveSupport #:nodoc:
                     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
                     
@@ -214,7 +231,7 @@ module ActiveSupport #:nodoc:
                     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) }
                   case value.length
                     when 0 then nil
                     when 1 then value.first
diff --git a/activesupport/test/core_ext/hash_ext_test.rb b/activesupport/test/core_ext/hash_ext_test.rb
index 44308c3..80873b4 100644
--- a/activesupport/test/core_ext/hash_ext_test.rb
+++ b/activesupport/test/core_ext/hash_ext_test.rb
@@ -575,12 +575,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
 
@@ -593,12 +591,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"]
@@ -612,7 +608,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
@@ -623,7 +618,6 @@ class HashToXmlTest < Test::Unit::TestCase
       :approved   => nil,
       :written_on => nil,
       :viewed_at  => nil,
-      :content    => nil, 
       :parent_id  => nil
     }.stringify_keys
 
@@ -833,6 +827,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]
@@ -867,7 +883,7 @@ class HashToXmlTest < Test::Unit::TestCase
   
   def test_empty_string_works_for_typecast_xml_value    
     assert_nothing_raised do
-      Hash.__send__(:typecast_xml_value, "")
+      Hash.__send__(:typecast_xml_value, "", [])
     end
   end
   
-- 
1.7.10.2 (Apple Git-33)