Attachment 591438 Details for Bug 831573 – Proposed upstream patch against the v2.3 branch

[patch] Proposed upstream patch against the v2.3 branch

2-3-sql-injection.patch (text/plain), 3.19 KB, created by Jan Lieskovsky on 2012-06-13 10:56:48 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jan Lieskovsky

Created: 2012-06-13 10:56:48 UTC

Size: 3.19 KB

patch

obsolete

>From 5dc350ceef0dfd8822218aec5edfbd18ed7ad0ed Mon Sep 17 00:00:00 2001
>From: Justin Collins <justin@presidentbeef.com>
>Date: Fri, 1 Jun 2012 19:02:11 -0700
>Subject: [PATCH] Fix SQL injection via nested hashes in conditions
>
>---
> activerecord/lib/active_record/base.rb |   10 ++++++----
> activerecord/test/cases/finder_test.rb |   16 ++++++++++++++++
> 2 files changed, 22 insertions(+), 4 deletions(-)
>
>diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
>index 461007f..d8a4daf 100755
>--- a/activerecord/lib/active_record/base.rb
>+++ b/activerecord/lib/active_record/base.rb
>@@ -2333,17 +2333,17 @@ module ActiveRecord #:nodoc:
>         # And for value objects on a composed_of relationship:
>         #   { :address => Address.new("123 abc st.", "chicago") }
>         #     # => "address_street='123 abc st.' and address_city='chicago'"
>-        def sanitize_sql_hash_for_conditions(attrs, default_table_name = quoted_table_name)
>+        def sanitize_sql_hash_for_conditions(attrs, default_table_name = quoted_table_name, top_level = true)
>           attrs = expand_hash_conditions_for_aggregates(attrs)
> 
>           conditions = attrs.map do |attr, value|
>             table_name = default_table_name
> 
>-            unless value.is_a?(Hash)
>+            if not value.is_a?(Hash)
>               attr = attr.to_s
> 
>               # Extract table name from qualified attribute names.
>-              if attr.include?('.')
>+              if attr.include?('.') and top_level
>                 attr_table_name, attr = attr.split('.', 2)
>                 attr_table_name = connection.quote_table_name(attr_table_name)
>               else
>@@ -2351,8 +2351,10 @@ module ActiveRecord #:nodoc:
>               end
> 
>               attribute_condition("#{attr_table_name}.#{connection.quote_column_name(attr)}", value)
>+            elsif top_level
>+              sanitize_sql_hash_for_conditions(value, connection.quote_table_name(attr.to_s), false)
>             else
>-              sanitize_sql_hash_for_conditions(value, connection.quote_table_name(attr.to_s))
>+              raise ActiveRecord::StatementInvalid
>             end
>           end.join(' AND ')
> 
>diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb
>index c779a69..a191f0f 100644
>--- a/activerecord/test/cases/finder_test.rb
>+++ b/activerecord/test/cases/finder_test.rb
>@@ -363,6 +363,22 @@ class FinderTest < ActiveRecord::TestCase
>     }
>   end
> 
>+  def test_hash_condition_find_with_improper_nested_hashes
>+    assert_raise(ActiveRecord::StatementInvalid) {
>+      Company.find(:first, :conditions => { :name => { :companies => { :id  => 1 }}})
>+    }
>+  end
>+
>+  def test_hash_condition_find_with_dot_in_nested_column_name
>+    assert_raise(ActiveRecord::StatementInvalid) {
>+      Company.find(:first, :conditions => { :name => { "companies.id" => 1 }})
>+    }
>+  end
>+
>+  def test_hash_condition_find_with_dot_in_column_name_okay
>+    assert Company.find(:first, :conditions => { "companies.id" => 1 })
>+  end
>+
>   def test_hash_condition_find_with_escaped_characters
>     Company.create("name" => "Ain't noth'n like' \#stuff")
>     assert Company.find(:first, :conditions => { :name => "Ain't noth'n like' \#stuff" })
>-- 
>1.7.4.4
>

Actions: View | Diff

Attachments on bug 831573: 591438 | 591439 | 591440 | 591441