From d4a53b2e02106c6734bbfea2a0e209febd5f36bd Mon Sep 17 00:00:00 2001 From: Tobias Kraze Date: Fri, 8 Feb 2013 12:52:10 +0100 Subject: [PATCH] fix serialization vulnerability --- .../lib/active_record/attribute_methods.rb | 17 ++++++++++++++++- activerecord/test/cases/base_test.rb | 6 ++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index 22630b3..ff71b42 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -80,7 +80,9 @@ module ActiveRecord end unless instance_method_already_implemented?("#{name}=") - if create_time_zone_conversion_attribute?(name, column) + if self.serialized_attributes[name] + define_write_method_for_serialized_attribute(name) + elsif create_time_zone_conversion_attribute?(name, column) define_write_method_for_time_zone_conversion(name) else define_write_method(name.to_sym) @@ -184,6 +186,19 @@ module ActiveRecord def define_write_method(attr_name) evaluate_attribute_method attr_name, "def #{attr_name}=(new_value);write_attribute('#{attr_name}', new_value);end", "#{attr_name}=" end + + # Defined for all serialized attributes. Disallows assigning already serialized YAML. + def define_write_method_for_serialized_attribute(attr_name) + method_body = <<-EOV + def #{attr_name}=(value) + if value.is_a?(String) and value =~ /^---/ + raise ActiveRecordError, "You tried to assign already serialized content to #{attr_name}. This is disabled due to security issues." + end + write_attribute(:#{attr_name}, value) + end + EOV + evaluate_attribute_method attr_name, method_body, "#{attr_name}=" + end # Defined for all +datetime+ and +timestamp+ attributes when +time_zone_aware_attributes+ are enabled. # This enhanced write method will automatically convert the time passed to it to the zone stored in Time.zone. diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 242be2a..f23894e 100755 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1499,6 +1499,12 @@ class BasicsTest < ActiveRecord::TestCase assert_nil topic.content end + def test_should_raise_exception_on_assigning_already_serialized_content + topic = Topic.new + serialized_content = %w[foo bar].to_yaml + assert_raise(ActiveRecord::ActiveRecordError) { topic.content = serialized_content } + end + def test_should_raise_exception_on_serialized_attribute_with_type_mismatch myobj = MyObject.new('value1', 'value2') topic = Topic.new(:content => myobj) -- 1.7.9.5