1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00

Don't allow where with non numeric string matches to 0 values

This is a follow-up of #35310.

Currently `Topic.find_by(id: "not-a-number")` matches to a `id = 0`
record. That is considered as silently leaking information.

If non numeric string is given to find by an integer column, it should
not be matched to any record.

Related #12793.
This commit is contained in:
Ryuta Kamizono 2019-02-20 20:55:09 +09:00
parent df2ebf9b59
commit 357cd23d3a
5 changed files with 23 additions and 9 deletions

View file

@ -26,15 +26,18 @@ module ActiveModel
private
def number_to_non_number?(old_value, new_value_before_type_cast)
old_value != nil && non_numeric_string?(new_value_before_type_cast)
old_value != nil && non_numeric_string?(new_value_before_type_cast.to_s)
end
def non_numeric_string?(value)
# 'wibble'.to_i will give zero, we want to make sure
# that we aren't marking int zero to string zero as
# changed.
!/\A[-+]?\d+/.match?(value.to_s)
!NUMERIC_REGEX.match?(value)
end
NUMERIC_REGEX = /\A\s*[+-]?\d/
private_constant :NUMERIC_REGEX
end
end
end

View file

@ -24,11 +24,8 @@ module ActiveModel
end
def serialize(value)
result = super
if result
ensure_in_range(result)
end
result
return if value.is_a?(::String) && non_numeric_string?(value)
ensure_in_range(super)
end
private
@ -39,9 +36,10 @@ module ActiveModel
end
def ensure_in_range(value)
unless range.cover?(value)
if value && !range.cover?(value)
raise ActiveModel::RangeError, "#{value} is out of range for #{self.class} with limit #{_limit} bytes"
end
value
end
def max_value

View file

@ -50,6 +50,14 @@ module ActiveModel
assert_equal 7200, type.cast(2.hours)
end
test "casting string for database" do
type = Type::Integer.new
assert_nil type.serialize("wibble")
assert_equal 5, type.serialize("5wibble")
assert_equal 5, type.serialize(" +5")
assert_equal(-5, type.serialize(" -5"))
end
test "casting empty string" do
type = Type::Integer.new
assert_nil type.cast("")

View file

@ -1,3 +1,7 @@
* Don't allow `where` with non numeric string matches to 0 values.
*Ryuta Kamizono*
* Introduce `ActiveRecord::Relation#destroy_by` and `ActiveRecord::Relation#delete_by`.
`destroy_by` allows relation to find all the records matching the condition and perform

View file

@ -51,8 +51,9 @@ module ActiveRecord
end
def test_where_with_invalid_value
topics(:first).update!(written_on: nil, bonus_time: nil, last_read: nil)
topics(:first).update!(parent_id: 0, written_on: nil, bonus_time: nil, last_read: nil)
assert_empty Topic.where(parent_id: Object.new)
assert_empty Topic.where(parent_id: "not-a-number")
assert_empty Topic.where(written_on: "")
assert_empty Topic.where(bonus_time: "")
assert_empty Topic.where(last_read: "")