[DRE-maint] Bug#789311: Bug#789311: ruby-rack: CVE-2015-3225: Potential Denial of Service Vulnerability in Rack normalize_params()
Salvatore Bonaccorso
carnil at debian.org
Thu Jul 30 19:58:27 UTC 2015
Hi,
(Adding Antonio to the loop who did the previous uploads)
On Thu, Jul 30, 2015 at 06:36:56PM +0900, Youhei SASAKI wrote:
> Hi,
>
> Thanks your review.
>
> On Thu, 30 Jul 2015 04:49:12 +0900,
> Salvatore Bonaccorso <carnil at debian.org> wrote:
> > >
> > > # BTW, due to the unreported FTBFS issue about ruby-rack in jessie, we
> > > # can't build package without "DH_RUBY_IGNORE_TESTS=all"...
> >
> > It builds for me here in pbuilder. Were exactly is the problem
> > located?
>
> In "lib/rack/response.rb": Upstream Issue: #631
> - https://github.com/rack/rack/issues/631
>
> I attached 0002-Fix-unreported-FTBFS.patch.
> This is aleady applied in unstable.
>
> > "patchwise" both looks okay but I have some small comments, first the
> > one for wheezy-security:
> - snip-
> > Use 1.4.1-2.1+deb7u1 as version, wheezy-security as distribution and
> > urgency=high.
> - snip-
> > Same here. use 1.5.2-3+deb8u1 as version, target jessie-security and
> > use urgency=high.
> - snip -
> > Could you make the above changes?
>
> Thanks. Update package version number and changelogs. debdiff attached.
The targetting distribution was still set to 'unstable'. I have fixed
that in the attached debdiffs and added the patch for jessie-security
(can you import them in your VCS please?). I have uploaded to
security-master the jessie-security one as attached. But for
wheezy-security the package does not built. Build-log is attached. It
fails for me as well already with 1.4.1-2.1. Can you have a look?
Regards,
Salvatore
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ruby-rack_1.4.1-2.1+deb7u1_amd64.build.gz
Type: application/gzip
Size: 5654 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-ruby-extras-maintainers/attachments/20150730/5228be2c/attachment.bin>
-------------- next part --------------
diff -Nru ruby-rack-1.4.1/debian/changelog ruby-rack-1.4.1/debian/changelog
--- ruby-rack-1.4.1/debian/changelog 2013-02-22 00:55:14.000000000 +0100
+++ ruby-rack-1.4.1/debian/changelog 2015-07-30 19:57:00.000000000 +0200
@@ -1,3 +1,14 @@
+ruby-rack (1.4.1-2.1+deb7u1) wheezy-security; urgency=high
+
+ * Create cherry-picked patch for Security Fix (Closes: #789311).
+ - CVE-2015-3225: 0006-Fix-Params_Depth.patch
+ Default depth at which the parameter parser will raise an exception
+ for being too deep, allows remote attackers to cause a denial of
+ service (SystemStackError) via a request with a large parameter
+ depth.
+
+ -- Youhei SASAKI <uwabami at gfd-dennou.org> Wed, 29 Jul 2015 16:37:25 +0900
+
ruby-rack (1.4.1-2.1) unstable; urgency=high
[ KURASHIKI Satoru ]
diff -Nru ruby-rack-1.4.1/debian/patches/0006-Fix-Params_Depth.patch ruby-rack-1.4.1/debian/patches/0006-Fix-Params_Depth.patch
--- ruby-rack-1.4.1/debian/patches/0006-Fix-Params_Depth.patch 1970-01-01 01:00:00.000000000 +0100
+++ ruby-rack-1.4.1/debian/patches/0006-Fix-Params_Depth.patch 2015-07-30 19:57:00.000000000 +0200
@@ -0,0 +1,88 @@
+From: Aaron Patterson <aaron.patterson () gmail com>
+Date: Tue, 20 Jan 2015 14:30:13 -0800
+Subject: raise an exception if the parameters are too deep
+
+CVE-2015-3225
+
+Conflicts:
+ lib/rack/utils.rb
+ test/spec_utils.rb
+---
+ lib/rack/utils.rb | 15 +++++++++++----
+ test/spec_utils.rb | 12 ++++++++++++
+ 2 files changed, 23 insertions(+), 4 deletions(-)
+
+diff --git a/lib/rack/utils.rb b/lib/rack/utils.rb
+index 6576dd2..4656f4a 100644
+--- a/lib/rack/utils.rb
++++ b/lib/rack/utils.rb
+@@ -49,12 +49,17 @@ module Rack
+
+ class << self
+ attr_accessor :key_space_limit
++ attr_accessor :param_depth_limit
+ end
+
+ # The default number of bytes to allow parameter keys to take up.
+ # This helps prevent a rogue client from flooding a Request.
+ self.key_space_limit = 65536
+
++ # Default depth at which the parameter parser will raise an exception for
++ # being too deep. This helps prevent SystemStackErrors
++ self.param_depth_limit = 100
++
+ # Stolen from Mongrel, with some small modifications:
+ # Parses a query string by breaking it up at the '&'
+ # and ';' characters. You can also use this to parse
+@@ -94,7 +99,9 @@ module Rack
+ end
+ module_function :parse_nested_query
+
+- def normalize_params(params, name, v = nil)
++ def normalize_params(params, name, v = nil, depth = Utils.param_depth_limit)
++ raise RangeError if depth <= 0
++
+ name =~ %r(\A[\[\]]*([^\[\]]+)\]*)
+ k = $1 || ''
+ after = $' || ''
+@@ -112,14 +119,14 @@ module Rack
+ params[k] ||= []
+ raise TypeError, "expected Array (got #{params[k].class.name}) for param `#{k}'" unless params[k].is_a?(Array)
+ if params_hash_type?(params[k].last) && !params[k].last.key?(child_key)
+- normalize_params(params[k].last, child_key, v)
++ normalize_params(params[k].last, child_key, v, depth - 1)
+ else
+- params[k] << normalize_params(params.class.new, child_key, v)
++ params[k] << normalize_params(params.class.new, child_key, v, depth - 1)
+ end
+ else
+ params[k] ||= params.class.new
+ raise TypeError, "expected Hash (got #{params[k].class.name}) for param `#{k}'" unless params_hash_type?(params[k])
+- params[k] = normalize_params(params[k], after, v)
++ params[k] = normalize_params(params[k], after, v, depth - 1)
+ end
+
+ return params
+diff --git a/test/spec_utils.rb b/test/spec_utils.rb
+index 69e3fbb..ac1e003 100644
+--- a/test/spec_utils.rb
++++ b/test/spec_utils.rb
+@@ -114,6 +114,18 @@ describe Rack::Utils do
+ Rack::Utils.parse_query("foo%3Dbaz=bar").should.equal "foo=baz" => "bar"
+ end
+
++ should "raise an exception if the params are too deep" do
++ len = Rack::Utils.param_depth_limit
++
++ lambda {
++ Rack::Utils.parse_nested_query("foo#{"[a]" * len}=bar")
++ }.should.raise(RangeError)
++
++ lambda {
++ Rack::Utils.parse_nested_query("foo#{"[a]" * (len - 1)}=bar")
++ }.should.not.raise
++ end
++
+ should "parse nested query strings correctly" do
+ Rack::Utils.parse_nested_query("foo").
+ should.equal "foo" => nil
diff -Nru ruby-rack-1.4.1/debian/patches/series ruby-rack-1.4.1/debian/patches/series
--- ruby-rack-1.4.1/debian/patches/series 2013-02-21 13:10:07.000000000 +0100
+++ ruby-rack-1.4.1/debian/patches/series 2015-07-30 19:57:00.000000000 +0200
@@ -3,3 +3,4 @@
0003-Reimplement-auth-scheme-fix.patch
0004-Prevent-symlink-path-traversals.patch
0005-Use-secure_compare-for-hmac-comparison.patch
+0006-Fix-Params_Depth.patch
-------------- next part --------------
diff -Nru ruby-rack-1.5.2/debian/changelog ruby-rack-1.5.2/debian/changelog
--- ruby-rack-1.5.2/debian/changelog 2014-10-17 14:44:22.000000000 +0200
+++ ruby-rack-1.5.2/debian/changelog 2015-07-30 19:44:25.000000000 +0200
@@ -1,3 +1,16 @@
+ruby-rack (1.5.2-3+deb8u1) jessie-security; urgency=high
+
+ * Create cherry-picked patch for Security Fix (Closes: #789311).
+ - CVE-2015-3225: 0001-Fix-Params_Depth.patch
+ Default depth at which the parameter parser will raise an exception
+ for being too deep, allows remote attackers to cause a denial of
+ service (SystemStackError) via a request with a large parameter
+ depth.
+ * Add 0002-Add-missing-require-to-response.rb.patch.
+ Add missing require of rack/body_proxy in response.rb
+
+ -- Youhei SASAKI <uwabami at gfd-dennou.org> Wed, 29 Jul 2015 17:12:00 +0900
+
ruby-rack (1.5.2-3) unstable; urgency=medium
* add myself to Uploaders:
diff -Nru ruby-rack-1.5.2/debian/patches/0001-Fix-Params_Depth.patch ruby-rack-1.5.2/debian/patches/0001-Fix-Params_Depth.patch
--- ruby-rack-1.5.2/debian/patches/0001-Fix-Params_Depth.patch 1970-01-01 01:00:00.000000000 +0100
+++ ruby-rack-1.5.2/debian/patches/0001-Fix-Params_Depth.patch 2015-07-30 19:44:25.000000000 +0200
@@ -0,0 +1,84 @@
+From: Aaron Patterson <aaron.patterson () gmail com>
+Date: Tue, 20 Jan 2015 14:30:13 -0800
+Subject: raise an exception if the parameters are too deep
+
+CVE-2015-3225
+
+Conflicts:
+ lib/rack/utils.rb
+ test/spec_utils.rb
+---
+ lib/rack/utils.rb | 15 +++++++++++----
+ test/spec_utils.rb | 12 ++++++++++++
+ 2 files changed, 23 insertions(+), 4 deletions(-)
+
+--- a/lib/rack/utils.rb
++++ b/lib/rack/utils.rb
+@@ -52,12 +52,17 @@
+
+ class << self
+ attr_accessor :key_space_limit
++ attr_accessor :param_depth_limit
+ end
+
+ # The default number of bytes to allow parameter keys to take up.
+ # This helps prevent a rogue client from flooding a Request.
+ self.key_space_limit = 65536
+
++ # Default depth at which the parameter parser will raise an exception for
++ # being too deep. This helps prevent SystemStackErrors
++ self.param_depth_limit = 100
++
+ # Stolen from Mongrel, with some small modifications:
+ # Parses a query string by breaking it up at the '&'
+ # and ';' characters. You can also use this to parse
+@@ -100,7 +105,9 @@
+ end
+ module_function :parse_nested_query
+
+- def normalize_params(params, name, v = nil)
++ def normalize_params(params, name, v = nil, depth = Utils.param_depth_limit)
++ raise RangeError if depth <= 0
++
+ name =~ %r(\A[\[\]]*([^\[\]]+)\]*)
+ k = $1 || ''
+ after = $' || ''
+@@ -118,14 +125,14 @@
+ params[k] ||= []
+ raise TypeError, "expected Array (got #{params[k].class.name}) for param `#{k}'" unless params[k].is_a?(Array)
+ if params_hash_type?(params[k].last) && !params[k].last.key?(child_key)
+- normalize_params(params[k].last, child_key, v)
++ normalize_params(params[k].last, child_key, v, depth - 1)
+ else
+- params[k] << normalize_params(params.class.new, child_key, v)
++ params[k] << normalize_params(params.class.new, child_key, v, depth - 1)
+ end
+ else
+ params[k] ||= params.class.new
+ raise TypeError, "expected Hash (got #{params[k].class.name}) for param `#{k}'" unless params_hash_type?(params[k])
+- params[k] = normalize_params(params[k], after, v)
++ params[k] = normalize_params(params[k], after, v, depth - 1)
+ end
+
+ return params
+--- a/test/spec_utils.rb
++++ b/test/spec_utils.rb
+@@ -123,6 +123,18 @@
+ Rack::Utils.parse_query(",foo=bar;,", ";,").should.equal "foo" => "bar"
+ end
+
++ should "raise an exception if the params are too deep" do
++ len = Rack::Utils.param_depth_limit
++
++ lambda {
++ Rack::Utils.parse_nested_query("foo#{"[a]" * len}=bar")
++ }.should.raise(RangeError)
++
++ lambda {
++ Rack::Utils.parse_nested_query("foo#{"[a]" * (len - 1)}=bar")
++ }.should.not.raise
++ end
++
+ should "parse nested query strings correctly" do
+ Rack::Utils.parse_nested_query("foo").
+ should.equal "foo" => nil
diff -Nru ruby-rack-1.5.2/debian/patches/0002-Add-missing-require-to-response.rb.patch ruby-rack-1.5.2/debian/patches/0002-Add-missing-require-to-response.rb.patch
--- ruby-rack-1.5.2/debian/patches/0002-Add-missing-require-to-response.rb.patch 1970-01-01 01:00:00.000000000 +0100
+++ ruby-rack-1.5.2/debian/patches/0002-Add-missing-require-to-response.rb.patch 2015-07-30 19:44:25.000000000 +0200
@@ -0,0 +1,24 @@
+From 24935a53d8ee561229a5e7e651be120ddda11562 Mon Sep 17 00:00:00 2001
+From: James Tucker <jftucker at gmail.com>
+Date: Sat, 28 Dec 2013 13:33:54 -0400
+Subject: [PATCH] Add missing require to response.rb
+
+Closes #631
+---
+ lib/rack/response.rb | 1 +
+ 1 file changed, 1 insertion(+)
+
+diff --git a/lib/rack/response.rb b/lib/rack/response.rb
+index 2076aff..1acfce2 100644
+--- a/lib/rack/response.rb
++++ b/lib/rack/response.rb
+@@ -1,5 +1,6 @@
+ require 'rack/request'
+ require 'rack/utils'
++require 'rack/body_proxy'
+ require 'time'
+
+ module Rack
+--
+2.5.0
+
diff -Nru ruby-rack-1.5.2/debian/patches/series ruby-rack-1.5.2/debian/patches/series
--- ruby-rack-1.5.2/debian/patches/series 1970-01-01 01:00:00.000000000 +0100
+++ ruby-rack-1.5.2/debian/patches/series 2015-07-30 19:44:25.000000000 +0200
@@ -0,0 +1,2 @@
+0001-Fix-Params_Depth.patch
+0002-Add-missing-require-to-response.rb.patch
More information about the Pkg-ruby-extras-maintainers
mailing list