Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date: Wed, 2 Jan 2013 13:22:22 -0800
From: Aaron Patterson <tenderlove@...y-lang.org>
To: rubyonrails-security@...glegroups.com, oss-security@...ts.openwall.com
Subject: SQL Injection Vulnerability in Ruby on Rails (CVE-2012-5664)

SQL Injection Vulnerability in Ruby on Rails

There is a SQL injection vulnerability in Active Record in ALL versions. This vulnerability has been assigned the CVE identifier CVE-2012-5664.

Versions Affected:  All.
Not affected:       NONE.
Fixed Versions:     3.2.10, 3.1.9, 3.0.18

Impact 
------ 
Due to the way dynamic finders in Active Record extract options from method parameters, a method parameter can mistakenly be used as a scope.  Carefully crafted requests can use the scope to inject arbitrary SQL.

All users running an affected release should either upgrade or use one of the work arounds immediately. 

Impacted code passes user provided data to a dynamic finder like this:

  Post.find_by_id(params[:id])

Releases 
-------- 
The  3.2.10, 3.1.9 & 3.0.18 releases are available at the normal locations. 

Workarounds 
----------- 
The issue can be mitigated by explicitly converting the parameter to an expected value.  For example, change this:

  Post.find_by_id(params[:id])

to this:

  Post.find_by_id(params[:id].to_s)


Patches 
------- 
To aid users who aren't able to upgrade immediately we have provided patches for the two supported release series and two unsupported versions.  They are in git-am format and consist of a single changeset. 

* 3-2-dynamic_finder_injection.patch - Patch for 3.2 series
* 3-1-dynamic_finder_injection.patch - Patch for 3.1 series
* 3-0-dynamic_finder_injection.patch - Patch for 3.0 series
* 2-3-dynamic_finder_injection.patch - Patch for 2.3 series

Please note that only the 3.1.x and 3.2.x series are supported at present.  Users of earlier unsupported releases are advised to upgrade as soon as possible as we cannot guarantee the continued availability of security fixes for unsupported releases.

-- 
Aaron Patterson
http://tenderlovemaking.com/

>From 9de9b359d0d24f70f0f6c5c58a7ad8750684d456 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@...il.com>
Date: Sun, 23 Dec 2012 11:07:07 -0800
Subject: [PATCH] CVE-2012-5664 options hashes should only be extracted if
 there are extra parameters

---
 activerecord/lib/active_record/base.rb |    6 +++++-
 activerecord/test/cases/finder_test.rb |   12 ++++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 461007f..809a38c 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -1897,7 +1897,11 @@ module ActiveRecord #:nodoc:
               # end
               self.class_eval <<-EOS, __FILE__, __LINE__ + 1
                 def self.#{method_id}(*args)
-                  options = args.extract_options!
+                  options = if args.length > #{attribute_names.size}
+                              args.extract_options!
+                            else
+                              {}
+                            end
                   attributes = construct_attributes_from_arguments(
                     [:#{attribute_names.join(',:')}],
                     args
diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb
index c779a69..9e3ab92 100644
--- a/activerecord/test/cases/finder_test.rb
+++ b/activerecord/test/cases/finder_test.rb
@@ -66,6 +66,18 @@ end
 class FinderTest < ActiveRecord::TestCase
   fixtures :companies, :topics, :entrants, :developers, :developers_projects, :posts, :comments, :accounts, :authors, :customers
 
+  def test_find_by_id_with_hash
+    assert_raises(ActiveRecord::StatementInvalid) do
+      Post.find_by_id(:limit => 1)
+    end
+  end
+
+  def test_find_by_title_and_id_with_hash
+    assert_raises(ActiveRecord::StatementInvalid) do
+      Post.find_by_title_and_id('foo', :limit => 1)
+    end
+  end
+
   def test_find
     assert_equal(topics(:first).title, Topic.find(1).title)
   end
-- 
1.7.10.2 (Apple Git-33)


>From 3542641ebd83a31f6b633b7af30ae6f37e563a1b Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@...il.com>
Date: Sun, 23 Dec 2012 11:07:07 -0800
Subject: [PATCH] CVE-2012-5664 options hashes should only be extracted if
 there are extra parameters

---
 activerecord/lib/active_record/base.rb |    6 +++++-
 activerecord/test/cases/finder_test.rb |   12 ++++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index f89b949..a05623d 100644
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -988,7 +988,11 @@ module ActiveRecord #:nodoc:
             attribute_names = match.attribute_names
             super unless all_attributes_exists?(attribute_names)
             if match.finder?
-              options = arguments.extract_options!
+              options = if arguments.length > attribute_names.size
+                          arguments.extract_options!
+                        else
+                          {}
+                        end
               relation = options.any? ? construct_finder_arel(options, current_scoped_methods) : scoped
               relation.send :find_by_attributes, match, attribute_names, *arguments
             elsif match.instantiator?
diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb
index 68b03a4..0705f15 100644
--- a/activerecord/test/cases/finder_test.rb
+++ b/activerecord/test/cases/finder_test.rb
@@ -65,6 +65,18 @@ end
 class FinderTest < ActiveRecord::TestCase
   fixtures :companies, :topics, :entrants, :developers, :developers_projects, :posts, :comments, :accounts, :authors, :customers, :categories, :categorizations
 
+  def test_find_by_id_with_hash
+    assert_raises(ActiveRecord::StatementInvalid) do
+      Post.find_by_id(:limit => 1)
+    end
+  end
+
+  def test_find_by_title_and_id_with_hash
+    assert_raises(ActiveRecord::StatementInvalid) do
+      Post.find_by_title_and_id('foo', :limit => 1)
+    end
+  end
+
   def test_find
     assert_equal(topics(:first).title, Topic.find(1).title)
   end
-- 
1.7.10.2 (Apple Git-33)


>From c42f548960fe2359571e62748bf7b92f4fa22f66 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@...il.com>
Date: Sun, 23 Dec 2012 11:07:07 -0800
Subject: [PATCH] CVE-2012-5664 options hashes should only be extracted if
 there are extra parameters

---
 activerecord/lib/active_record/base.rb |    6 +++++-
 activerecord/test/cases/finder_test.rb |   12 ++++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index aafb48d..ffa4a48 100644
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -1065,7 +1065,11 @@ Calling dynamic finder with less number of arguments than the number of attribut
                 eowarn
             end
             if match.finder?
-              options = arguments.extract_options!
+              options = if arguments.length > attribute_names.size
+                          arguments.extract_options!
+                        else
+                          {}
+                        end
               relation = options.any? ? scoped(options) : scoped
               relation.send :find_by_attributes, match, attribute_names, *arguments
             elsif match.instantiator?
diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb
index bab9857..546f7ee 100644
--- a/activerecord/test/cases/finder_test.rb
+++ b/activerecord/test/cases/finder_test.rb
@@ -15,6 +15,18 @@ require 'models/toy'
 class FinderTest < ActiveRecord::TestCase
   fixtures :companies, :topics, :entrants, :developers, :developers_projects, :posts, :comments, :accounts, :authors, :customers, :categories, :categorizations
 
+  def test_find_by_id_with_hash
+    assert_raises(ActiveRecord::StatementInvalid) do
+      Post.find_by_id(:limit => 1)
+    end
+  end
+
+  def test_find_by_title_and_id_with_hash
+    assert_raises(ActiveRecord::StatementInvalid) do
+      Post.find_by_title_and_id('foo', :limit => 1)
+    end
+  end
+
   def test_find
     assert_equal(topics(:first).title, Topic.find(1).title)
   end
-- 
1.7.10.2 (Apple Git-33)


>From 325669f0795a9148fd31f7f496a40dc8e114ef52 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@...il.com>
Date: Sun, 23 Dec 2012 11:07:07 -0800
Subject: [PATCH] CVE-2012-5664 options hashes should only be extracted if
 there are extra parameters

---
 activerecord/lib/active_record/dynamic_matchers.rb |    7 ++++++-
 activerecord/test/cases/finder_test.rb             |   12 ++++++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/activerecord/lib/active_record/dynamic_matchers.rb b/activerecord/lib/active_record/dynamic_matchers.rb
index b6b8e24..f15d0b7 100644
--- a/activerecord/lib/active_record/dynamic_matchers.rb
+++ b/activerecord/lib/active_record/dynamic_matchers.rb
@@ -40,7 +40,12 @@ module ActiveRecord
           METHOD
           send(method_id, *arguments)
         elsif match.finder?
-          options = arguments.extract_options!
+          options = if arguments.length > attribute_names.size
+                      arguments.extract_options!
+                    else
+                      {}
+                    end
+
           relation = options.any? ? scoped(options) : scoped
           relation.send :find_by_attributes, match, attribute_names, *arguments, &block
         elsif match.instantiator?
diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb
index 5d72e35..77ca3b5 100644
--- a/activerecord/test/cases/finder_test.rb
+++ b/activerecord/test/cases/finder_test.rb
@@ -15,6 +15,18 @@ require 'models/toy'
 class FinderTest < ActiveRecord::TestCase
   fixtures :companies, :topics, :entrants, :developers, :developers_projects, :posts, :comments, :accounts, :authors, :customers, :categories, :categorizations
 
+  def test_find_by_id_with_hash
+    assert_raises(ActiveRecord::StatementInvalid) do
+      Post.find_by_id(:limit => 1)
+    end
+  end
+
+  def test_find_by_title_and_id_with_hash
+    assert_raises(ActiveRecord::StatementInvalid) do
+      Post.find_by_title_and_id('foo', :limit => 1)
+    end
+  end
+
   def test_find
     assert_equal(topics(:first).title, Topic.find(1).title)
   end
-- 
1.7.10.2 (Apple Git-33)


Powered by blists - more mailing lists

Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.

Powered by Openwall GNU/*/Linux - Powered by OpenVZ