Skip to content
This repository
Browse code

Fix SQL injection via nested hashes in conditions

  • Loading branch information...
commit 62f81f4d6b3ee40e9887ffd92ab14714bad93f18 1 parent e8c0597
authored June 01, 2012 Mina Naguib committed June 13, 2012
10  activerecord/lib/active_record/base.rb
@@ -2333,17 +2333,17 @@ def expand_hash_conditions_for_aggregates(attrs)
2333 2333
         # And for value objects on a composed_of relationship:
2334 2334
         #   { :address => Address.new("123 abc st.", "chicago") }
2335 2335
         #     # => "address_street='123 abc st.' and address_city='chicago'"
2336  
-        def sanitize_sql_hash_for_conditions(attrs, default_table_name = quoted_table_name)
  2336
+        def sanitize_sql_hash_for_conditions(attrs, default_table_name = quoted_table_name, top_level = true)
2337 2337
           attrs = expand_hash_conditions_for_aggregates(attrs)
2338 2338
 
2339 2339
           conditions = attrs.map do |attr, value|
2340 2340
             table_name = default_table_name
2341 2341
 
2342  
-            unless value.is_a?(Hash)
  2342
+            if not value.is_a?(Hash)
2343 2343
               attr = attr.to_s
2344 2344
 
2345 2345
               # Extract table name from qualified attribute names.
2346  
-              if attr.include?('.')
  2346
+              if attr.include?('.') and top_level
2347 2347
                 attr_table_name, attr = attr.split('.', 2)
2348 2348
                 attr_table_name = connection.quote_table_name(attr_table_name)
2349 2349
               else
@@ -2351,8 +2351,10 @@ def sanitize_sql_hash_for_conditions(attrs, default_table_name = quoted_table_na
2351 2351
               end
2352 2352
 
2353 2353
               attribute_condition("#{attr_table_name}.#{connection.quote_column_name(attr)}", value)
  2354
+            elsif top_level
  2355
+              sanitize_sql_hash_for_conditions(value, connection.quote_table_name(attr.to_s), false)
2354 2356
             else
2355  
-              sanitize_sql_hash_for_conditions(value, connection.quote_table_name(attr.to_s))
  2357
+              raise ActiveRecord::StatementInvalid
2356 2358
             end
2357 2359
           end.join(' AND ')
2358 2360
 
16  activerecord/test/cases/finder_test.rb
@@ -363,6 +363,22 @@ def test_hash_condition_find_malformed
363 363
     }
364 364
   end
365 365
 
  366
+  def test_hash_condition_find_with_improper_nested_hashes
  367
+    assert_raise(ActiveRecord::StatementInvalid) {
  368
+      Company.find(:first, :conditions => { :name => { :companies => { :id  => 1 }}})
  369
+    }
  370
+  end
  371
+
  372
+  def test_hash_condition_find_with_dot_in_nested_column_name
  373
+    assert_raise(ActiveRecord::StatementInvalid) {
  374
+      Company.find(:first, :conditions => { :name => { "companies.id" => 1 }})
  375
+    }
  376
+  end
  377
+
  378
+  def test_hash_condition_find_with_dot_in_column_name_okay
  379
+    assert Company.find(:first, :conditions => { "companies.id" => 1 })
  380
+  end
  381
+
366 382
   def test_hash_condition_find_with_escaped_characters
367 383
     Company.create("name" => "Ain't noth'n like' \#stuff")
368 384
     assert Company.find(:first, :conditions => { :name => "Ain't noth'n like' \#stuff" })

0 notes on commit 62f81f4

Please sign in to comment.
Something went wrong with that request. Please try again.