Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [day] [month] [year] [list]
Date: Thu, 11 Aug 2016 10:53:17 -0700
From: Aaron Patterson <tenderlove@...y-lang.org>
To: security@...e.de, rubyonrails-security@...glegroups.com,
	oss-security@...ts.openwall.com, ruby-security-ann@...glegroups.com
Subject: [CVE-2016-6317] Unsafe Query Generation Risk in Active Record

# Unsafe Query Generation Risk in Active Record

There is a vulnerability when Active Record is used in conjunction with JSON
parameter parsing. This vulnerability has been assigned the CVE identifier
CVE-2016-6317.  This vulnerability is similar to CVE-2012-2660, CVE-2012-2694
and CVE-2013-0155.

Versions Affected:  >= 4.2.0
Not affected:       < 4.2.0, >= 5.0.0
Fixed Versions:     4.2.7.1

Impact
------

Due to the way Active Record interprets parameters in combination with the way that JSON parameters are parsed, it is possible for an attacker to issue unexpected database queries with "IS NULL" or empty where clauses.  This issue does *not* let an attacker insert arbitrary values into an SQL query, however they can cause the query to check for NULL or eliminate a WHERE clause when most users wouldn't expect it. 

For example, a system has password reset with token functionality: 

    unless params[:token].nil? 
      user = User.find_by_token(params[:token]) 
      user.reset_password! 
    end 

An attacker can craft a request such that `params[:token]` will return `[nil]`.  The `[nil]` value will bypass the test for nil, but will still add an "IN ('xyz', NULL)" clause to the SQL query. 

Similarly, an attacker can craft a request such that `params[:token]` will return an empty hash.  An empty hash will eliminate the WHERE clause of the query, but can bypass the `nil?` check. 

Note that this impacts not only dynamic finders (`find_by_*`) but also relations (`User.where(:name => params[:name])`). 

All users running an affected release should either upgrade or use one of the work arounds immediately. All users running an affected release should upgrade immediately. Please note, this vulnerability is a variant of CVE-2012-2660, CVE-2012-2694, and CVE-2013-0155.  Even if you upgraded to address those issues, you must take action again. 

If this chance in behavior impacts your application, you can manually decode the original values from the request like so: 

    ActiveSupport::JSON.decode(request.body) 

Releases
--------
The FIXED releases are available at the normal locations. 

Workarounds
-----------
This problem can be mitigated by casting the parameter to a string before passing it to Active Record.  For example: 

    unless params[:token].nil? || params[:token].to_s.empty? 
      user = User.find_by_token(params[:token].to_s) 
      user.reset_password! 
    end 


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

* 4-2-unsafe-query-generation.patch - Patch for 4.2 series

Please note that only the 5.0.x and 4.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.

Credits
-------

Thanks to joernchen of Phenoelit for reporting this!

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

From 04dd9752926e41b3e1d7b6a357f1f4381aaeece9 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@...il.com>
Date: Thu, 4 Aug 2016 11:15:03 -0700
Subject: [PATCH 2/2] Fix unsafe query generation risk.

Redo of CVE-2012-2660, CVE-2012-2694 and CVE-2013-0155

CVE-2016-6317
---
 .../dispatch/request/json_params_parsing_test.rb   | 43 ++++++++++++++++++++++
 .../relation/predicate_builder/array_handler.rb    |  3 +-
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/actionpack/test/dispatch/request/json_params_parsing_test.rb b/actionpack/test/dispatch/request/json_params_parsing_test.rb
index c609075..e8dec17 100644
--- a/actionpack/test/dispatch/request/json_params_parsing_test.rb
+++ b/actionpack/test/dispatch/request/json_params_parsing_test.rb
@@ -84,7 +84,50 @@ class JsonParamsParsingTest < ActionDispatch::IntegrationTest
     end
   end
 
+  test "prevent null query" do
+    # Make sure we have data to find
+    klass = Class.new(ActiveRecord::Base) do
+      def self.name; 'Foo'; end
+      establish_connection adapter: "sqlite3", database: ":memory:"
+      connection.create_table "foos" do |t|
+        t.string :title
+        t.timestamps null: false
+      end
+    end
+    klass.create
+    assert klass.first
+
+    app = ActionDispatch::ParamsParser.new ->(env) {
+      request = ActionDispatch::Request.new env
+      params = ActionController::Parameters.new request.parameters
+      if params[:t]
+        klass.find_by_title(params[:t])
+      else
+        nil
+      end
+    }
+
+    assert_nil app.call(make_env({ 't' => nil }))
+    assert_nil app.call(make_env({ 't' => [nil] }))
+
+    [[[nil]], [[[nil]]]].each do |data|
+      assert_deprecated do
+        assert_nil app.call(make_env({ 't' => data }))
+      end
+    end
+  end
+
   private
+    def make_env json
+      data = JSON.dump json
+      content_length = data.length
+      {
+        'CONTENT_LENGTH' => content_length,
+        'CONTENT_TYPE'   => 'application/json',
+        'rack.input'     => StringIO.new(data)
+      }
+    end
+
     def assert_parses(expected, actual, headers = {})
       with_test_routing do
         post "/parse", actual, headers
diff --git a/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb
index fb08326..d4e74eb 100644
--- a/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb
+++ b/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb
@@ -14,7 +14,8 @@ module ActiveRecord
             it for 'IN' conditions.
           MSG
 
-          values = values.flatten
+          flat_values = values.flatten
+          values = flat_values unless flat_values.include?(nil)
         end
 
         return attribute.in([]) if values.empty? && nils.empty?
-- 
2.8.1



[ CONTENT OF TYPE application/pgp-signature SKIPPED ]

Powered by blists - more mailing lists

Your e-mail address:

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

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