Date: Sat, 2 Jun 2012 01:58:30 -0400 From: Eitan Adler <lists@eitanadler.com> To: ruby@freebsd.org Cc: ports-security@freebsd.org Subject: Fwd: [oss-security] Unsafe Query Generation Risk in Ruby on Rails (CVE-2012-2660) Message-ID: <CAF6rxgku5-4QOWNuwZyPesMZZLvLcZHFUW_bzSA=avKtVPk11A@mail.gmail.com> In-Reply-To: <20120531191529.GB79783@higgins.local> References: <20120531191529.GB79783@higgins.local>
index | next in thread | previous in thread | raw e-mail
[-- Attachment #1 --] A vulnerability has been found in a port you maintain. Please commit an update and write up a VuXML report. If you need help feel free to email ports-security@freebsd.org, ---------- Forwarded message ---------- From: Aaron Patterson <tenderlove@ruby-lang.org> Date: 31 May 2012 15:15 Subject: [oss-security] Unsafe Query Generation Risk in Ruby on Rails (CVE-2012-2660) To: oss-security@lists.openwall.com Unsafe Query Generation Risk in Ruby on Rails There is a vulnerability when Active Record is used in conjunction with parameter parsing from Rack via Action Pack. This vulnerability has been assigned the CVE identifier CVE-2012-2660. Versions Affected: ALL versions Not affected: NONE Fixed Versions: 3.2.4, 3.1.5, 3.0.13 Impact ------ Due to the way Active Record interprets parameters in combination with the way that Rack parses query parameters, it is possible for an attacker to issue unexpected database queries with "IS NULL" 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 where 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 "IS NULL" clause to the SQL query. All users running an affected release should either upgrade or use one of the work arounds immediately. Releases -------- The FIXED releases are available at the normal locations. Workarounds ----------- This problem can be mitigated by testing for `[nil]`. For example: unless params[:token].nil? || params[:token] == [nil] user = User.find_by_token(params[:token]) user.reset_password! end Another possible workaround is to cast to a known type and test against that type. For example: unless params[:token].to_s.empty? user = User.find_by_token(params[:token]) 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. * 3-0-null_param.patch - Patch for 3.0 series * 3-1-null_param.patch - Patch for 3.1 series * 3-2-null_param.patch - Patch for 3.2 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. Credits ------- Thanks to Ben Murphy for reporting the vulnerability to us, and to Chad Pyne of thoughtbot for helping us verify the fix. -- Aaron Patterson http://tenderlovemaking.com/ -- Eitan Adler [-- Attachment #2 --] From c202638225519b5e1a03ebe523b109c948fb0e52 Mon Sep 17 00:00:00 2001 From: Aaron Patterson <aaron.patterson@gmail.com> Date: Wed, 30 May 2012 15:13:03 -0700 Subject: [PATCH] Strip [nil] from parameters hash. Thanks to Ben Murphy for reporting this! CVE-2012-2660 Conflicts: actionpack/lib/action_dispatch/http/request.rb --- actionpack/lib/action_dispatch/http/request.rb | 22 ++++++++++++++++++++ .../dispatch/request/query_string_parsing_test.rb | 7 +++++- 2 files changed, 28 insertions(+), 1 deletions(-) diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 7c8557b..985b730 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -257,5 +257,27 @@ module ActionDispatch def local? LOCALHOST.any? { |local_ip| local_ip === remote_addr && local_ip === remote_ip } end + + protected + + # Remove nils from the params hash + def deep_munge(hash) + hash.each_value do |v| + case v + when Array + v.grep(Hash) { |x| deep_munge(x) } + when Hash + deep_munge(v) + end + end + + keys = hash.keys.find_all { |k| hash[k] == [nil] } + keys.each { |k| hash[k] = nil } + hash + end + + def parse_query(qs) + deep_munge(super) + end end end diff --git a/actionpack/test/dispatch/request/query_string_parsing_test.rb b/actionpack/test/dispatch/request/query_string_parsing_test.rb index 071d80c..c7ab700 100644 --- a/actionpack/test/dispatch/request/query_string_parsing_test.rb +++ b/actionpack/test/dispatch/request/query_string_parsing_test.rb @@ -81,7 +81,12 @@ class QueryStringParsingTest < ActionController::IntegrationTest end test "query string without equal" do - assert_parses({ "action" => nil }, "action") + assert_parses({"action" => nil}, "action") + assert_parses({"action" => {"foo" => nil}}, "action[foo]") + assert_parses({"action" => {"foo" => { "bar" => nil }}}, "action[foo][bar]") + assert_parses({"action" => {"foo" => { "bar" => nil }}}, "action[foo][bar][]") + assert_parses({"action" => {"foo" => nil}}, "action[foo][]") + assert_parses({"action"=>{"foo"=>[{"bar"=>nil}]}}, "action[foo][][bar]") end test "query string with empty key" do -- 1.7.5.4 [-- Attachment #3 --] From 5b83bbfab7d5770ed56366d739ff62ac70425008 Mon Sep 17 00:00:00 2001 From: Aaron Patterson <aaron.patterson@gmail.com> Date: Wed, 30 May 2012 15:13:03 -0700 Subject: [PATCH] Strip [nil] from parameters hash. Thanks to Ben Murphy for reporting this! CVE-2012-2660 --- actionpack/lib/action_dispatch/http/request.rb | 22 ++++++++++++++++++++ .../dispatch/request/query_string_parsing_test.rb | 7 +++++- 2 files changed, 28 insertions(+), 1 deletions(-) diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index c49cc56..9e1dc4b 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -267,6 +267,28 @@ module ActionDispatch LOCALHOST.any? { |local_ip| local_ip === remote_addr && local_ip === remote_ip } end + protected + + # Remove nils from the params hash + def deep_munge(hash) + hash.each_value do |v| + case v + when Array + v.grep(Hash) { |x| deep_munge(x) } + when Hash + deep_munge(v) + end + end + + keys = hash.keys.find_all { |k| hash[k] == [nil] } + keys.each { |k| hash[k] = nil } + hash + end + + def parse_query(qs) + deep_munge(super) + end + private def check_method(name) diff --git a/actionpack/test/dispatch/request/query_string_parsing_test.rb b/actionpack/test/dispatch/request/query_string_parsing_test.rb index f6a1475..181f51a 100644 --- a/actionpack/test/dispatch/request/query_string_parsing_test.rb +++ b/actionpack/test/dispatch/request/query_string_parsing_test.rb @@ -81,7 +81,12 @@ class QueryStringParsingTest < ActionDispatch::IntegrationTest end test "query string without equal" do - assert_parses({ "action" => nil }, "action") + assert_parses({"action" => nil}, "action") + assert_parses({"action" => {"foo" => nil}}, "action[foo]") + assert_parses({"action" => {"foo" => { "bar" => nil }}}, "action[foo][bar]") + assert_parses({"action" => {"foo" => { "bar" => nil }}}, "action[foo][bar][]") + assert_parses({"action" => {"foo" => nil}}, "action[foo][]") + assert_parses({"action"=>{"foo"=>[{"bar"=>nil}]}}, "action[foo][][bar]") end test "query string with empty key" do -- 1.7.5.4 [-- Attachment #4 --] From dff6db18840e2fd1dd3f3e4ef0ae7a9a3986d01d Mon Sep 17 00:00:00 2001 From: Aaron Patterson <aaron.patterson@gmail.com> Date: Wed, 30 May 2012 15:13:03 -0700 Subject: [PATCH] Strip [nil] from parameters hash. Thanks to Ben Murphy for reporting this! CVE-2012-2660 --- actionpack/lib/action_dispatch/http/request.rb | 22 ++++++++++++++++++++ .../dispatch/request/query_string_parsing_test.rb | 7 +++++- 2 files changed, 28 insertions(+), 1 deletions(-) diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 8209212..adbb5d1 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -247,6 +247,28 @@ module ActionDispatch LOCALHOST.any? { |local_ip| local_ip === remote_addr && local_ip === remote_ip } end + protected + + # Remove nils from the params hash + def deep_munge(hash) + hash.each_value do |v| + case v + when Array + v.grep(Hash) { |x| deep_munge(x) } + when Hash + deep_munge(v) + end + end + + keys = hash.keys.find_all { |k| hash[k] == [nil] } + keys.each { |k| hash[k] = nil } + hash + end + + def parse_query(qs) + deep_munge(super) + end + private def check_method(name) diff --git a/actionpack/test/dispatch/request/query_string_parsing_test.rb b/actionpack/test/dispatch/request/query_string_parsing_test.rb index f6a1475..181f51a 100644 --- a/actionpack/test/dispatch/request/query_string_parsing_test.rb +++ b/actionpack/test/dispatch/request/query_string_parsing_test.rb @@ -81,7 +81,12 @@ class QueryStringParsingTest < ActionDispatch::IntegrationTest end test "query string without equal" do - assert_parses({ "action" => nil }, "action") + assert_parses({"action" => nil}, "action") + assert_parses({"action" => {"foo" => nil}}, "action[foo]") + assert_parses({"action" => {"foo" => { "bar" => nil }}}, "action[foo][bar]") + assert_parses({"action" => {"foo" => { "bar" => nil }}}, "action[foo][bar][]") + assert_parses({"action" => {"foo" => nil}}, "action[foo][]") + assert_parses({"action"=>{"foo"=>[{"bar"=>nil}]}}, "action[foo][][bar]") end test "query string with empty key" do -- 1.7.5.4 [-- Attachment #5 --] -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (Darwin) iQEcBAEBAgAGBQJPx8NRAAoJEJUxcLy0/6/G2V0IAJQ9gc/Sv91mhdGm4c0JiEUo 8mwsO8SX6s3KPwxuIcjtJ0OyBVpx1MiO82yeaedGvFxzJwVBTG0vwn2gAF++I821 CKZqmeuFXTR2Xwpe6z5mDq9lXaqX4sp6a8nDFRskSNJFw7yPELPzU3V6qnRybzGt 29P4t1iOmtUAs0m5j8loiczHxWzLrdVWL2MInrObZ4kE0fn70EgV0Uh3UFN6A8KQ 8FTG2+9tYa/lckZAUh4mHCMyS1pYcWVUV7z9sJsnbra/J+GEY68/skwjftvbf3YU +di9maFXHHlwOQn1uBqgDVunXAWbje0fIvOHeGdjHX+63lDJ6kuY57PtnBdpJkQ= =/D8g -----END PGP SIGNATURE-----help
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAF6rxgku5-4QOWNuwZyPesMZZLvLcZHFUW_bzSA=avKtVPk11A>