Fix SQL injection via nested hashes in conditions

Some pretty gnarly bugs and security issues are present in the
latest rack 1.1.x release. There are 1.2.x and 1.3.x releases that
correct these.

This changes the gem dependencies to allow for rack versions > 1.1.
At GitHub we're on 1.2.4 (latest 1.2.x release at present) and
should have some results from real world testing soon.

actionpack/Rakefile

@@ -79,7 +79,7 @@ spec = Gem::Specification.new do |s|
   s.requirements << 'none'
 
   s.add_dependency('activesupport', '= 2.3.14' + PKG_BUILD)
-  s.add_dependency('rack', '~> 1.1.3')
+  s.add_dependency('rack', '~> 1.1')
 
   s.require_path = 'lib'
 

actionpack/lib/action_controller.rb

@@ -31,7 +31,7 @@ rescue LoadError
   end
 end
 
-gem 'rack', '~> 1.1.3'
+gem 'rack', '~> 1.1'
 require 'rack'
 require 'action_controller/cgi_ext'
 

actionpack/lib/action_controller/request.rb

@@ -225,7 +225,7 @@ module ActionController
         not_trusted_addrs = remote_addr_list.reject {|addr| addr =~ TRUSTED_PROXIES}
         return not_trusted_addrs.first unless not_trusted_addrs.empty?
       end
-      remote_ips = @env['HTTP_X_FORWARDED_FOR'].present? && @env['HTTP_X_FORWARDED_FOR'].split(',')
+      remote_ips = @env['HTTP_X_FORWARDED_FOR'] && @env['HTTP_X_FORWARDED_FOR'].split(',')
 
       if @env.include? 'HTTP_CLIENT_IP'
         if ActionController::Base.ip_spoofing_check && remote_ips && !remote_ips.include?(@env['HTTP_CLIENT_IP'])

actionpack/test/controller/request_test.rb

@@ -20,9 +20,6 @@ class RequestTest < ActiveSupport::TestCase
       'HTTP_X_FORWARDED_FOR' => '3.4.5.6'
     assert_equal '1.2.3.4', request.remote_ip
 
-    request = stub_request 'HTTP_X_FORWARDED_FOR' => ''
-    assert_nil request.remote_ip
-
     request = stub_request 'REMOTE_ADDR' => '127.0.0.1',
       'HTTP_X_FORWARDED_FOR' => '3.4.5.6'
     assert_equal '3.4.5.6', request.remote_ip

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 ')
 

activerecord/lib/active_record/validations.rb

@@ -602,14 +602,14 @@ module ActiveRecord
       # Validates that the specified attribute matches the length restrictions supplied. Only one option can be used at a time:
       #
       #   class Person < ActiveRecord::Base
-      #     validates_length_of :first_name, :maximum => 30
-      #     validates_length_of :last_name, :maximum => 30, :message => "less than %{count} if you don't mind"
+      #     validates_length_of :first_name, :maximum=>30
+      #     validates_length_of :last_name, :maximum=>30, :message=>"less than %{count} if you don't mind"
       #     validates_length_of :fax, :in => 7..32, :allow_nil => true
       #     validates_length_of :phone, :in => 7..32, :allow_blank => true
       #     validates_length_of :user_name, :within => 6..20, :too_long => "pick a shorter name", :too_short => "pick a longer name"
-      #     validates_length_of :zip_code, :minimum => 5, :too_short => "please enter at least %{count} characters"
-      #     validates_length_of :smurf_leader, :is => 4, :message => "papa is spelled with %{count} characters... don't play me"
-      #     validates_length_of :essay, :minimum => 100, :too_short => "Your essay must be at least %{count} words"), :tokenizer => lambda {|str| str.scan(/\w+/) }
+      #     validates_length_of :fav_bra_size, :minimum => 1, :too_short => "please enter at least %{count} character"
+      #     validates_length_of :smurf_leader, :is => 4, :message => "papa is spelled with %{count} characters... don't play me."
+      #     validates_length_of :essay, :minimum => 100, :too_short => "Your essay must be at least %{count} words."), :tokenizer => lambda {|str| str.scan(/\w+/) }
       #   end
       #
       # Configuration options:

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" })