Andreas Schwarz
2006-Jul-30 09:27 UTC
[Security] validates_numericality_of can be tricked into accepting anything
(I would have posted this to the tracker, but I get an error message when I try to create a ticket.) validates_numericality_of uses the regular expression /^[\+\-]?\d+$/, but because ^ and $ match the start/end of a line, as long as one line in the input string matches the value is accepted. The solution is to use \A and \Z instead of ^ and $. A patch is attached. _______________________________________________ Rails-core mailing list Rails-core@lists.rubyonrails.org http://lists.rubyonrails.org/mailman/listinfo/rails-core
Hampton
2006-Jul-30 22:25 UTC
Re: [Security] validates_numericality_of can be tricked into accepting anything
Nice catch! +1 from me! -hampton. On 7/30/06, Andreas Schwarz <usenet@andreas-s.net> wrote:> (I would have posted this to the tracker, but I get an error message > when I try to create a ticket.) > > validates_numericality_of uses the regular expression /^[\+\-]?\d+$/, > but because ^ and $ match the start/end of a line, as long as one line > in the input string matches the value is accepted. The solution is to > use \A and \Z instead of ^ and $. > > A patch is attached. > > > Index: activerecord/test/validations_test.rb > ==================================================================> --- activerecord/test/validations_test.rb (revision 4630) > +++ activerecord/test/validations_test.rb (working copy) > @@ -1013,7 +1013,7 @@ > end > > > -class ValidatesNumericalityTest > +class ValidatesNumericalityTest < Test::Unit::TestCase > NIL = [nil, "", " ", " \t \r \n"] > BIGDECIMAL_STRINGS = %w(12345678901234567890.1234567890) # 30 significent digits > FLOAT_STRINGS = %w(0.0 +0.0 -0.0 10.0 10.5 -10.5 -0.0001 -090.1 90.1e1 -90.1e5 -90.1e-5 90e-5) > @@ -1021,7 +1021,7 @@ > FLOATS = [0.0, 10.0, 10.5, -10.5, -0.0001] + FLOAT_STRINGS > INTEGERS = [0, 10, -10] + INTEGER_STRINGS > BIGDECIMAL = BIGDECIMAL_STRINGS.collect! { |bd| BigDecimal.new(bd) } > - JUNK = ["not a number", "42 not a number", "0xdeadbeef", "00-1", "--3", "+-3", "+3-1", "-+019.0", "12.12.13.12"] > + JUNK = ["not a number", "42 not a number", "0xdeadbeef", "00-1", "--3", "+-3", "+3-1", "-+019.0", "12.12.13.12", "123\nnot a number"] > > def setup > Topic.write_inheritable_attribute(:validate, nil) > @@ -1061,7 +1061,7 @@ > def invalid!(values) > values.each do |value| > topic = Topic.create("title" => "numeric test", "content" => "whatever", "approved" => value) > - assert !topic.valid?, "#{value} not rejected as a number" > + assert !topic.valid?, "#{value.inspect} not rejected as a number" > assert topic.errors.on(:approved) > end > end > @@ -1069,7 +1069,7 @@ > def valid!(values) > values.each do |value| > topic = Topic.create("title" => "numeric test", "content" => "whatever", "approved" => value) > - assert topic.valid?, "#{value} not accepted as a number" > + assert topic.valid?, "#{value.inspect} not accepted as a number" > end > end > end > Index: activerecord/lib/active_record/validations.rb > ==================================================================> --- activerecord/lib/active_record/validations.rb (revision 4630) > +++ activerecord/lib/active_record/validations.rb (working copy) > @@ -554,9 +554,11 @@ > # provided. > # > # class Person < ActiveRecord::Base > - # validates_format_of :email, :with => /^([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})$/i, :on => :create > + # validates_format_of :email, :with => /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/i, :on => :create > # end > # > + # Note: use \A and \Z to match the start and end of the string, ^ and $ match the start/end of a line. > + # > # A regular expression must be provided or else an exception will be raised. > # > # Configuration options: > @@ -670,7 +672,7 @@ > > # Validates whether the value of the specified attribute is numeric by trying to convert it to > # a float with Kernel.Float (if <tt>integer</tt> is false) or applying it to the regular expression > - # <tt>/^[\+\-]?\d+$/</tt> (if <tt>integer</tt> is set to true). > + # <tt>/\A[\+\-]?\d+\Z/</tt> (if <tt>integer</tt> is set to true). > # > # class Person < ActiveRecord::Base > # validates_numericality_of :value, :on => :create > @@ -691,7 +693,7 @@ > > if configuration[:only_integer] > validates_each(attr_names,configuration) do |record, attr_name,value| > - record.errors.add(attr_name, configuration[:message]) unless record.send("#{attr_name}_before_type_cast").to_s =~ /^[+-]?\d+$/ > + record.errors.add(attr_name, configuration[:message]) unless record.send("#{attr_name}_before_type_cast").to_s =~ /\A[+-]?\d+\Z/ > end > else > validates_each(attr_names,configuration) do |record, attr_name,value| > > > _______________________________________________ > Rails-core mailing list > Rails-core@lists.rubyonrails.org > http://lists.rubyonrails.org/mailman/listinfo/rails-core > > >