diff options
| -rw-r--r-- | activerecord/CHANGELOG | 10 | ||||
| -rwxr-xr-x | activerecord/lib/active_record/base.rb | 26 | ||||
| -rwxr-xr-x | activerecord/test/finder_test.rb | 7 |
3 files changed, 32 insertions, 11 deletions
diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 0d19cf3e44..0b0c0048e9 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,15 @@ *CVS* +* Added bind-style variable interpolation for the condition arrays that uses the adapter's quote method [Michael Koziarski] + + Before: + find_first([ "user_name = '%s' AND password = '%s'", user_name, password ])] + find_first([ "firm_id = %s", firm_id ])] # unsafe! + + After: + find_first([ "user_name = ? AND password = ?", user_name, password ])] + find_first([ "firm_id = ?", firm_id ])] + * Added CSV format for fixtures #272 [what-a-day]. (See the new and expanded documentation on fixtures for more information) * Fixed fixtures using primary key fields called something else than "id" [dave] diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 1ebc843274..a45480945e 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -67,7 +67,7 @@ module ActiveRecord #:nodoc: # end # # def self.authenticate_safely(user_name, password) - # find_first([ "user_name = '%s' AND password = '%s'", user_name, password ]) + # find_first([ "user_name = ? AND password = ?", user_name, password ]) # end # end # @@ -76,10 +76,6 @@ module ActiveRecord #:nodoc: # on the other hand, will sanitize the <tt>user_name</tt> and +password+ before inserting them in the query, which will ensure that # an attacker can't escape the query and fake the login (or worse). # - # Beware, that the approach used in <tt>authenticate_unsafely</tt> is basically just a wrapped call to sprintf. This means that you - # still have to quote when using %s or use %d instead. So find_first([ "firm_id = %s", firm_id ]) is _not_ safe while both - # find_first([ "firm_id = '%s'", firm_id ]) and find_first([ "firm_id = %d", firm_id ]) are. - # # == Overwriting default accessors # # All column values are automatically available through basic accessors on the Active Record object, but some times you @@ -636,13 +632,21 @@ module ActiveRecord #:nodoc: # Accepts either a condition array or string. The string is returned untouched, but the array has each of # the condition values sanitized. def sanitize_conditions(conditions) - if Array === conditions - statement, values = conditions[0], conditions[1..-1] - values.collect! { |value| sanitize(value) } - conditions = statement % values + return conditions unless conditions.is_a?(Array) + + statement, values = conditions[0], conditions[1..-1] + + statement =~ /\?/ ? + replace_bind_variables(statement, values) : + statement % values.collect { |value| sanitize(value) } + end + + def replace_bind_variables(statement, values) + while statement =~ /\?/ + statement.sub!(/\?/, connection.quote(values.shift)) end - - return conditions + + return statement end end diff --git a/activerecord/test/finder_test.rb b/activerecord/test/finder_test.rb index d369f6b033..b7b4ab589a 100755 --- a/activerecord/test/finder_test.rb +++ b/activerecord/test/finder_test.rb @@ -60,6 +60,13 @@ class FinderTest < Test::Unit::TestCase assert_kind_of Time, Topic.find_first(["id = %d", 1]).written_on end + def test_bind_variables + assert_kind_of Firm, Company.find_first(["name = ?", "37signals"]) + assert_nil Company.find_first(["name = ?", "37signals!"]) + assert_nil Company.find_first(["name = ?", "37signals!' OR 1=1"]) + assert_kind_of Time, Topic.find_first(["id = ?", 1]).written_on + end + def test_string_sanitation assert_equal "something '' 1=1", ActiveRecord::Base.sanitize("something ' 1=1") assert_equal "something select table", ActiveRecord::Base.sanitize("something; select table") |