mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
prevent users from unknowingly using bad regexps that can compromise security (http://homakov.blogspot.co.uk/2012/05/saferweb-injects-in-various-ruby.html)
This commit is contained in:
parent
f278b06789
commit
bc7c0b5c10
6 changed files with 67 additions and 18 deletions
|
@ -37,6 +37,11 @@
|
|||
|
||||
* Trim down Active Model API by removing `valid?` and `errors.full_messages` *José Valim*
|
||||
|
||||
* When `^` or `$` are used in the regular expression provided to `validates_format_of` and the :multiline option is not set to true, an exception will be raised. This is to prevent security vulnerabilities when using `validates_format_of`. The problem is described in detail in the Rails security guide.
|
||||
|
||||
## Rails 3.2.6 (Jun 12, 2012) ##
|
||||
|
||||
* No changes.
|
||||
|
||||
## Rails 3.2.5 (Jun 1, 2012) ##
|
||||
|
||||
|
|
|
@ -32,11 +32,21 @@ module ActiveModel
|
|||
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)
|
||||
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) &&
|
||||
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
|
||||
|
@ -47,7 +57,7 @@ module ActiveModel
|
|||
# attribute matches the regular expression:
|
||||
#
|
||||
# class Person < ActiveRecord::Base
|
||||
# validates_format_of :email, :with => /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/i, :on => :create
|
||||
# validates_format_of :email, :with => /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\z/i, :on => :create
|
||||
# end
|
||||
#
|
||||
# Alternatively, you can require that the specified attribute does _not_
|
||||
|
@ -63,12 +73,16 @@ module ActiveModel
|
|||
# class Person < ActiveRecord::Base
|
||||
# # Admin can have number as a first letter in their screen name
|
||||
# validates_format_of :screen_name,
|
||||
# :with => lambda{ |person| person.admin? ? /\A[a-z0-9][a-z0-9_\-]*\Z/i : /\A[a-z][a-z0-9_\-]*\Z/i }
|
||||
# :with => lambda{ |person| person.admin? ? /\A[a-z0-9][a-z0-9_\-]*\z/i : /\A[a-z][a-z0-9_\-]*\z/i }
|
||||
# end
|
||||
#
|
||||
# Note: use <tt>\A</tt> and <tt>\Z</tt> to match the start and end of the
|
||||
# string, <tt>^</tt> and <tt>$</tt> match the start/end of a line.
|
||||
#
|
||||
# Due to frequent misuse of <tt>^</tt> and <tt>$</tt>, you need to pass the
|
||||
# :multiline => true option in case you use any of these two anchors in the provided
|
||||
# regular expression. In most cases, you should be using <tt>\A</tt> and <tt>\z</tt>.
|
||||
#
|
||||
# You must pass either <tt>:with</tt> or <tt>:without</tt> as an option.
|
||||
# In addition, both must be a regular expression or a proc or lambda, or
|
||||
# else an exception will be raised.
|
||||
|
@ -98,6 +112,9 @@ module ActiveModel
|
|||
# method, proc or string should return or evaluate to a true or false value.
|
||||
# * <tt>:strict</tt> - Specifies whether validation should be strict.
|
||||
# See <tt>ActiveModel::Validation#validates!</tt> for more information.
|
||||
# * <tt>:multiline</tt> - Set to true if your regular expression contains
|
||||
# anchors that match the beginning or end of lines as opposed to the
|
||||
# beginning or end of the string. These anchors are <tt>^</tt> and <tt>$</tt>.
|
||||
def validates_format_of(*attr_names)
|
||||
validates_with FormatValidator, _merge_attributes(attr_names)
|
||||
end
|
||||
|
|
|
@ -11,7 +11,7 @@ class PresenceValidationTest < ActiveModel::TestCase
|
|||
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 => /\AValidation\smacros \w+!\z/, :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 @@ class PresenceValidationTest < ActiveModel::TestCase
|
|||
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 => /\AValidation\smacros \w+!\z/, :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 @@ class PresenceValidationTest < ActiveModel::TestCase
|
|||
|
||||
# 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 => /\A[1-9][0-9]*\z/, :message => "is bad data")
|
||||
|
||||
t = Topic.new("title" => "72x", "content" => "6789")
|
||||
assert t.invalid?, "Shouldn't be valid"
|
||||
|
@ -63,11 +63,21 @@ class PresenceValidationTest < ActiveModel::TestCase
|
|||
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 => /\AValid Title\z/, :message => "can't be %{value}")
|
||||
t = Topic.new(:title => 'Invalid title')
|
||||
assert t.invalid?
|
||||
assert_equal ["can't be Invalid title"], 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_with_not_option
|
||||
Topic.validates_format_of(:title, :without => /foo/, :message => "should not contain foo")
|
||||
|
|
|
@ -141,7 +141,7 @@ class I18nValidationTest < ActiveModel::TestCase
|
|||
|
||||
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 => /\A[1-9][0-9]*\z/)
|
||||
@person.title = '72x'
|
||||
@person.errors.expects(:generate_message).with(:title, :invalid, generate_message_options.merge(:value => '72x'))
|
||||
@person.valid?
|
||||
|
@ -291,7 +291,7 @@ class I18nValidationTest < ActiveModel::TestCase
|
|||
# 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 => /\A[1-9][0-9]*\z/)
|
||||
end
|
||||
|
||||
# validates_inclusion_of w/o mocha
|
||||
|
|
|
@ -187,7 +187,7 @@ class Person
|
|||
attr_accessor :name, :email, :token
|
||||
|
||||
validates :name, :presence => true
|
||||
validates_format_of :email, :with => /^([^\s]+)((?:[-a-z0-9]\.)[a-z]{2,})$/i
|
||||
validates_format_of :email, :with => /\A([^\s]+)((?:[-a-z0-9]\.)[a-z]{2,})\z/i
|
||||
validates! :token, :presence => true
|
||||
|
||||
end
|
||||
|
|
|
@ -588,26 +588,43 @@ h4. Regular Expressions
|
|||
|
||||
INFO: _A common pitfall in Ruby's regular expressions is to match the string's beginning and end by ^ and $, instead of \A and \z._
|
||||
|
||||
Ruby uses a slightly different approach than many other languages to match the end and the beginning of a string. That is why even many Ruby and Rails books make this wrong. So how is this a security threat? Imagine you have a File model and you validate the file name by a regular expression like this:
|
||||
Ruby uses a slightly different approach than many other languages to match the end and the beginning of a string. That is why even many Ruby and Rails books make this wrong. So how is this a security threat? Say you wanted to loosely validate a URL field and you used a simple regular expression like this:
|
||||
|
||||
<ruby>
|
||||
class File < ActiveRecord::Base
|
||||
validates :name, :format => /^[\w\.\-\<plus>]<plus>$/
|
||||
end
|
||||
/^https?:\/\/[^\n]+$/i
|
||||
</ruby>
|
||||
|
||||
This means, upon saving, the model will validate the file name to consist only of alphanumeric characters, dots, + and -. And the programmer added ^ and $ so that file name will contain these characters from the beginning to the end of the string. However, _(highlight)in Ruby ^ and $ matches the *line* beginning and line end_. And thus a file name like this passes the filter without problems:
|
||||
This may work fine in some languages. However, _(highlight)in Ruby ^ and $ match the *line* beginning and line end_. And thus a URL like this passes the filter without problems:
|
||||
|
||||
<plain>
|
||||
file.txt%0A<script>alert('hello')</script>
|
||||
javascript:exploit_code();/*
|
||||
http://hi.com
|
||||
*/
|
||||
</plain>
|
||||
|
||||
Whereas %0A is a line feed in URL encoding, so Rails automatically converts it to "file.txt\n<script>alert('hello')</script>". This file name passes the filter because the regular expression matches – up to the line end, the rest does not matter. The correct expression should read:
|
||||
This URL passes the filter because the regular expression matches – the second line, the rest does not matter. Now imagine we had a view that showed the URL like this:
|
||||
|
||||
<ruby>
|
||||
/\A[\w\.\-\<plus>]<plus>\z/
|
||||
link_to "Homepage", @user.homepage
|
||||
</ruby>
|
||||
|
||||
The link looks innocent to visitors, but when it's clicked, it will execute the javascript function "exploit_code" or any other javascript the attacker provides.
|
||||
|
||||
To fix the regular expression, \A and \z should be used instead of ^ and $, like so:
|
||||
|
||||
<ruby>
|
||||
/\Ahttps?:\/\/[^\n]+\z/i
|
||||
</ruby>
|
||||
|
||||
Since this is a frequent mistake, the format validator (validates_format_of) now raises an exception if the provided regular expression starts with ^ or ends with $. If you do need to use ^ and $ instead of \A and \z (which is rare), you can set the :multiline option to true, like so:
|
||||
|
||||
<ruby>
|
||||
# content should include a line "Meanwhile" anywhere in the string
|
||||
validates :content, :format => { :with => /^Meanwhile$/, :multiline => true }
|
||||
</ruby>
|
||||
|
||||
Note that this only protects you against the most common mistake when using the format validator - you always need to keep in mind that ^ and $ match the *line* beginning and line end in Ruby, and not the beginning and end of a string.
|
||||
|
||||
h4. Privilege Escalation
|
||||
|
||||
WARNING: _Changing a single parameter may give the user unauthorized access. Remember that every parameter may be changed, no matter how much you hide or obfuscate it._
|
||||
|
|
Loading…
Reference in a new issue