Permalink
Browse files

prevent users from unknowingly using bad regexps that can compromise …

  • Loading branch information...
1 parent 08477a6 commit 8ad72d524e6b0fb9e77f931e94047e62fd62878a @mrbrdo mrbrdo committed May 31, 2012
@@ -23,7 +23,7 @@ def check_validity!
end
private
-
+
def option_call(record, name)
option = options[name]
option.respond_to?(:call) ? option.call(record) : option
@@ -32,11 +32,20 @@ def option_call(record, name)
def record_error(record, attribute, name, value)
record.errors.add(attribute, :invalid, options.except(name).merge!(:value => value))
end
+
+ def regexp_using_multiline_anchors?(regexp)
+ # only check for most common cases
+ regexp.source.start_with?("^") ||
+ (regexp.source.end_with?("$") && !regexp.source.end_with?("\\$"))
+ end
def check_options_validity(options, name)
option = options[name]
if option && !option.is_a?(Regexp) && !option.respond_to?(:call)
raise ArgumentError, "A regular expression or a proc or lambda must be supplied as :#{name}"
+ elsif option && option.is_a?(Regexp) && # check for multiline regexp
+ regexp_using_multiline_anchors?(option) && options[:multiline] != true
+ raise ArgumentError, "The provided regular expression is using multiline anchors (^ or $), which may present a security risk. Did you mean to use \\A and \\Z, or forgot to add the :multiline => true option?"
end
end
end
@@ -11,7 +11,7 @@ def teardown
end
def test_validate_format
- Topic.validates_format_of(:title, :content, :with => /^Validation\smacros \w+!$/, :message => "is bad data")
+ Topic.validates_format_of(:title, :content, :with => /^Validation\smacros \w+!$/, :multiline => true, :message => "is bad data")
t = Topic.new("title" => "i'm incorrect", "content" => "Validation macros rule!")
assert t.invalid?, "Shouldn't be valid"
@@ -27,7 +27,7 @@ def test_validate_format
end
def test_validate_format_with_allow_blank
- Topic.validates_format_of(:title, :with => /^Validation\smacros \w+!$/, :allow_blank => true)
+ Topic.validates_format_of(:title, :with => /^Validation\smacros \w+!$/, :multiline => true, :allow_blank => true)
assert Topic.new("title" => "Shouldn't be valid").invalid?
assert Topic.new("title" => "").valid?
assert Topic.new("title" => nil).valid?
@@ -36,7 +36,7 @@ def test_validate_format_with_allow_blank
# testing ticket #3142
def test_validate_format_numeric
- Topic.validates_format_of(:title, :content, :with => /^[1-9][0-9]*$/, :message => "is bad data")
+ Topic.validates_format_of(:title, :content, :with => /^[1-9][0-9]*$/, :multiline => true, :message => "is bad data")
t = Topic.new("title" => "72x", "content" => "6789")
assert t.invalid?, "Shouldn't be valid"
@@ -63,7 +63,7 @@ def test_validate_format_numeric
end
def test_validate_format_with_formatted_message
- Topic.validates_format_of(:title, :with => /^Valid Title$/, :message => "can't be %{value}")
+ Topic.validates_format_of(:title, :with => /^Valid Title$/, :multiline => true, :message => "can't be %{value}")
t = Topic.new(:title => 'Invalid title')
assert t.invalid?
assert_equal ["can't be Invalid title"], t.errors[:title]
@@ -81,6 +81,16 @@ def test_validate_format_with_not_option
t.valid?
assert_equal [], t.errors[:title]
end
+
+ def test_validate_format_of_with_multiline_regexp_should_raise_error
+ assert_raise(ArgumentError) { Topic.validates_format_of(:title, :with => /^Valid Title$/) }
+ end
+
+ def test_validate_format_of_with_multiline_regexp_and_option
+ assert_nothing_raised(ArgumentError) do
+ Topic.validates_format_of(:title, :with => /^Valid Title$/, :multiline => true)
+ end
+ end
def test_validate_format_of_without_any_regexp_should_raise_error
assert_raise(ArgumentError) { Topic.validates_format_of(:title) }
@@ -141,9 +141,9 @@ def test_errors_full_messages_uses_format
COMMON_CASES.each do |name, validation_options, generate_message_options|
test "validates_format_of on generated message #{name}" do
- Person.validates_format_of :title, validation_options.merge(:with => /^[1-9][0-9]*$/)
+ Person.validates_format_of :title, validation_options.merge(:with => /^[1-9][0-9]*$/, :multiline => true)
@person.title = '72x'
- @person.errors.expects(:generate_message).with(:title, :invalid, generate_message_options.merge(:value => '72x'))
+ @person.errors.expects(:generate_message).with(:title, :invalid, generate_message_options.merge(:value => '72x', :multiline => true))
@person.valid?
end
end
@@ -291,7 +291,7 @@ def self.set_expectations_for_validation(validation, error_type, &block_that_set
# validates_format_of w/o mocha
set_expectations_for_validation "validates_format_of", :invalid do |person, options_to_merge|
- Person.validates_format_of :title, options_to_merge.merge(:with => /^[1-9][0-9]*$/)
+ Person.validates_format_of :title, options_to_merge.merge(:with => /^[1-9][0-9]*$/, :multiline => true)
end
# validates_inclusion_of w/o mocha

0 comments on commit 8ad72d5

Please sign in to comment.