Browse files

fix serialization vulnerability

  • Loading branch information...
1 parent 9a48f4c commit 5cfe8330dc3bf2ef110cefed0c56f394dc699407 @kratob kratob committed with tenderlove Feb 8, 2013
Showing with 22 additions and 1 deletion.
  1. +16 −1 activerecord/lib/active_record/attribute_methods.rb
  2. +6 −0 activerecord/test/cases/base_test.rb
View
17 activerecord/lib/active_record/attribute_methods.rb
@@ -80,7 +80,9 @@ def define_attribute_methods
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 @@ def define_question_method(attr_name)
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.
View
6 activerecord/test/cases/base_test.rb
@@ -1499,6 +1499,12 @@ def test_nil_serialized_attribute_with_class_constraint
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)

2 comments on commit 5cfe833

@grosser

looks like that would mean enter "---" into a textfield -> boom oO

@grosser

Ok, only for serialized values, so not really a problem.

Please sign in to comment.