[DRE-maint] Bug#882034: ruby-redis-store for jessie and stretch (#882034 CVE-2017-1000248). Proposed patch
Cédric Boutillier
boutil at debian.org
Fri Dec 1 21:44:22 UTC 2017
Hi,
I have prepared a patch for Debian bug #882034 (CVE-2017-1000248) from
by adapting the upstream patch from
https://github.com/redis-store/redis-store/pull/290
(which should be applied after
https://github.com/redis-store/redis-store/commit/bcd1c28cf10ff18b4352cdacbe04113af3fec68d,
not present in the version 1.1.6)
Please find attached the debdiff for the version in Stretch.
It is the same as the change for 1.1.6-2 which went to unstable (without
the additional packaging change).
As jessie has the same version, the debdiff will look the same except
the one line in the changelog with version number and suite.
Do you ack this patch, and allow me to upload to security.debian.org?
Thanks
Cédric
-------------- next part --------------
diff -Nru ruby-redis-store-1.1.6/debian/changelog ruby-redis-store-1.1.6/debian/changelog
--- ruby-redis-store-1.1.6/debian/changelog 2015-07-28 13:52:12.000000000 +0200
+++ ruby-redis-store-1.1.6/debian/changelog 2017-12-01 17:22:29.000000000 +0100
@@ -1,3 +1,11 @@
+ruby-redis-store (1.1.6-1+deb9u1) stretch-security; urgency=high
+
+ * Team upload
+ * Add upstream patch to fix CVE-2017-1000248, allowing unsafe objects to be
+ loaded from redis (Closes: #882034)
+
+ -- Cédric Boutillier <boutil at debian.org> Fri, 01 Dec 2017 17:22:29 +0100
+
ruby-redis-store (1.1.6-1) unstable; urgency=medium
* Upstream update
diff -Nru ruby-redis-store-1.1.6/debian/patches/CVE-2017-1000248.patch ruby-redis-store-1.1.6/debian/patches/CVE-2017-1000248.patch
--- ruby-redis-store-1.1.6/debian/patches/CVE-2017-1000248.patch 1970-01-01 01:00:00.000000000 +0100
+++ ruby-redis-store-1.1.6/debian/patches/CVE-2017-1000248.patch 2017-12-01 17:21:20.000000000 +0100
@@ -0,0 +1,551 @@
+Description: Replace marshalling with pluggable serializers
+Author: Tom Scott <tscott at weblinc.com>
+Bug: https://github.com/redis-store/redis-store/issues/289
+Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=882034
+Applied-Upstream: https://github.com/redis-store/redis-store/commit/e0c1398d54a9661c8c70267c3a925ba6b192142e
+Origin: upstream
+Last-Update: Tue, 15 Aug 2017 11:07:07 -0400
+
+This is in response to a vulnerability warning we received on Friday,
+August 11th, 2017. While most users will not be affected by this
+change, we recommend that developers of new applications use a different
+serializer other than `Marshal`. This, along with the removal of the
+`:marshalling` option, will enforce "sane defaults" in terms of securely
+serializing/de-serializing data.
+
+- Add `:serializer` option and deprecate `:marshalling`. Although you
+ will still be able to enable/disable serialization with Marshal using
+ `:marshalling` in the 1.x series, this will be removed by 2.0.
+
+- Rename `Redis::Store::Marshalling` to `Redis::Store::Serialization` to
+ reflect its new purpose.
+
+Fixes #289
+---
+ lib/redis-store.rb | 12 -------
+ lib/redis/store.rb | 28 +++++++++++++--
+ lib/redis/store/factory.rb | 9 ++++-
+ lib/redis/store/namespace.rb | 4 +--
+ .../store/{marshalling.rb => serialization.rb} | 6 ++--
+ test/redis/store/factory_test.rb | 40 ++++++++++++++++++++--
+ test/redis/store/namespace_test.rb | 4 +--
+ .../{marshalling_test.rb => serialization_test.rb} | 4 +--
+ 8 files changed, 80 insertions(+), 27 deletions(-)
+ rename lib/redis/store/{marshalling.rb => serialization.rb} (90%)
+ rename test/redis/store/{marshalling_test.rb => serialization_test.rb} (98%)
+
+--- a/lib/redis-store.rb
++++ b/lib/redis-store.rb
+@@ -1,12 +1 @@
+-require 'redis'
+ require 'redis/store'
+-require 'redis/store/factory'
+-require 'redis/distributed_store'
+-require 'redis/store/namespace'
+-require 'redis/store/marshalling'
+-require 'redis/store/version'
+-
+-class Redis
+- class Store < self
+- end
+-end
+--- a/lib/redis/store.rb
++++ b/lib/redis/store.rb
+@@ -1,3 +1,9 @@
++require 'redis'
++require 'redis/store/factory'
++require 'redis/distributed_store'
++require 'redis/store/namespace'
++require 'redis/store/serialization'
++require 'redis/store/version'
+ require 'redis/store/ttl'
+ require 'redis/store/interface'
+
+@@ -7,6 +13,24 @@
+
+ def initialize(options = { })
+ super
++
++ unless options[:marshalling].nil?
++ puts %(
++ DEPRECATED: You are passing the :marshalling option, which has been
++ replaced with `serializer: Marshal` to support pluggable serialization
++ backends. To disable serialization (much like disabling marshalling),
++ pass `serializer: nil` in your configuration.
++
++ The :marshalling option will be removed for redis-store 2.0.
++ )
++ end
++
++ @serializer = options.key?(:serializer) ? options[:serializer] : Marshal
++
++ unless options[:marshalling].nil?
++ @serializer = options[:marshalling] ? Marshal : nil
++ end
++
+ _extend_marshalling options
+ _extend_namespace options
+ end
+@@ -22,8 +46,7 @@
+
+ private
+ def _extend_marshalling(options)
+- @marshalling = !(options[:marshalling] === false) # HACK - TODO delegate to Factory
+- extend Marshalling if @marshalling
++ extend Serialization unless @serializer.nil?
+ end
+
+ def _extend_namespace(options)
+--- a/lib/redis/store/marshalling.rb
++++ /dev/null
+@@ -1,56 +0,0 @@
+-class Redis
+- class Store < self
+- module Marshalling
+- def set(key, value, options = nil)
+- _marshal(value, options) { |value| super encode(key), encode(value), options }
+- end
+-
+- def setnx(key, value, options = nil)
+- _marshal(value, options) { |value| super encode(key), encode(value), options }
+- end
+-
+- def setex(key, expiry, value, options = nil)
+- _marshal(value, options) { |value| super encode(key), expiry, encode(value), options }
+- end
+-
+- def get(key, options = nil)
+- _unmarshal super(key), options
+- end
+-
+- def mget(*keys)
+- options = keys.pop if keys.last.is_a?(Hash)
+- super(*keys).map do |result|
+- _unmarshal result, options
+- end
+- end
+-
+- private
+- def _marshal(val, options)
+- yield marshal?(options) ? Marshal.dump(val) : val
+- end
+-
+- def _unmarshal(val, options)
+- unmarshal?(val, options) ? Marshal.load(val) : val
+- end
+-
+- def marshal?(options)
+- !(options && options[:raw])
+- end
+-
+- def unmarshal?(result, options)
+- result && result.size > 0 && marshal?(options)
+- end
+-
+- if defined?(Encoding)
+- def encode(string)
+- key = string.to_s.dup
+- key.force_encoding(Encoding::BINARY)
+- end
+- else
+- def encode(string)
+- string
+- end
+- end
+- end
+- end
+-end
+--- /dev/null
++++ b/lib/redis/store/serialization.rb
+@@ -0,0 +1,56 @@
++class Redis
++ class Store < self
++ module Serialization
++ def set(key, value, options = nil)
++ _marshal(value, options) { |value| super encode(key), encode(value), options }
++ end
++
++ def setnx(key, value, options = nil)
++ _marshal(value, options) { |value| super encode(key), encode(value), options }
++ end
++
++ def setex(key, expiry, value, options = nil)
++ _marshal(value, options) { |value| super encode(key), expiry, encode(value), options }
++ end
++
++ def get(key, options = nil)
++ _unmarshal super(key), options
++ end
++
++ def mget(*keys)
++ options = keys.pop if keys.last.is_a?(Hash)
++ super(*keys).map do |result|
++ _unmarshal result, options
++ end
++ end
++
++ private
++ def _marshal(val, options)
++ yield marshal?(options) ? @serializer.dump(val) : val
++ end
++
++ def _unmarshal(val, options)
++ unmarshal?(val, options) ? @serializer.load(val) : val
++ end
++
++ def marshal?(options)
++ !(options && options[:raw])
++ end
++
++ def unmarshal?(result, options)
++ result && result.size > 0 && marshal?(options)
++ end
++
++ if defined?(Encoding)
++ def encode(string)
++ key = string.to_s.dup
++ key.force_encoding(Encoding::BINARY)
++ end
++ else
++ def encode(string)
++ string
++ end
++ end
++ end
++ end
++end
+--- a/test/redis/store/factory_test.rb
++++ b/test/redis/store/factory_test.rb
+@@ -1,4 +1,5 @@
+ require 'test_helper'
++require 'json'
+
+ describe "Redis::Store::Factory" do
+ describe ".create" do
+@@ -41,9 +42,11 @@
+ store.instance_variable_get(:@client).password.must_equal("secret")
+ end
+
+- it "allows/disable marshalling" do
+- store = Redis::Store::Factory.create :marshalling => false
+- store.instance_variable_get(:@marshalling).must_equal(false)
++
++ it "disables serialization" do
++ store = Redis::Store::Factory.create :serializer => nil
++ store.instance_variable_get(:@serializer).must_be_nil
++ store.instance_variable_get(:@options)[:raw].must_equal(true)
+ end
+
+ it "should instantiate a Redis::DistributedStore store" do
+--- a/test/redis/store/namespace_test.rb
++++ b/test/redis/store/namespace_test.rb
+@@ -3,7 +3,7 @@
+ describe "Redis::Store::Namespace" do
+ def setup
+ @namespace = "theplaylist"
+- @store = Redis::Store.new :namespace => @namespace, :marshalling => false # TODO remove mashalling option
++ @store = Redis::Store.new :namespace => @namespace, :serializer => nil
+ @client = @store.instance_variable_get(:@client)
+ @rabbit = "bunny"
+ @default_store = Redis::Store.new
+@@ -77,7 +77,7 @@
+ end
+
+ describe 'method calls' do
+- let(:store){Redis::Store.new :namespace => @namespace, :marshalling => false}
++ let(:store){Redis::Store.new :namespace => @namespace, :serializer => nil}
+ let(:client){store.instance_variable_get(:@client)}
+
+ it "should namespace get" do
+--- a/test/redis/store/marshalling_test.rb
++++ /dev/null
+@@ -1,128 +0,0 @@
+-require 'test_helper'
+-
+-describe "Redis::Marshalling" do
+- def setup
+- @store = Redis::Store.new :marshalling => true
+- @rabbit = OpenStruct.new :name => "bunny"
+- @white_rabbit = OpenStruct.new :color => "white"
+- @store.set "rabbit", @rabbit
+- @store.del "rabbit2"
+- end
+-
+- def teardown
+- @store.flushdb
+- @store.quit
+- end
+-
+- it "unmarshals on get" do
+- @store.get("rabbit").must_equal(@rabbit)
+- end
+-
+- it "marshals on set" do
+- @store.set "rabbit", @white_rabbit
+- @store.get("rabbit").must_equal(@white_rabbit)
+- end
+-
+- if RUBY_VERSION.match /1\.9/
+- it "doesn't unmarshal on get if raw option is true" do
+- @store.get("rabbit", :raw => true).must_equal("\x04\bU:\x0FOpenStruct{\x06:\tnameI\"\nbunny\x06:\x06EF")
+- end
+- else
+- it "doesn't unmarshal on get if raw option is true" do
+- @store.get("rabbit", :raw => true).must_include("\x04\bU:\x0FOpenStruct{\x06:\tname")
+- end
+- end
+-
+- it "doesn't marshal set if raw option is true" do
+- @store.set "rabbit", @white_rabbit, :raw => true
+- @store.get("rabbit", :raw => true).must_equal(%(#<OpenStruct color="white">))
+- end
+-
+- it "doesn't unmarshal if get returns an empty string" do
+- @store.set "empty_string", ""
+- @store.get("empty_string").must_equal("")
+- # TODO use a meaningful Exception
+- # lambda { @store.get("empty_string").must_equal("") }.wont_raise Exception
+- end
+-
+- it "doesn't set an object if already exist" do
+- @store.setnx "rabbit", @white_rabbit
+- @store.get("rabbit").must_equal(@rabbit)
+- end
+-
+- it "marshals on set unless exists" do
+- @store.setnx "rabbit2", @white_rabbit
+- @store.get("rabbit2").must_equal(@white_rabbit)
+- end
+-
+- it "doesn't marshal on set unless exists if raw option is true" do
+- @store.setnx "rabbit2", @white_rabbit, :raw => true
+- @store.get("rabbit2", :raw => true).must_equal(%(#<OpenStruct color="white">))
+- end
+-
+- it "marshals on set expire" do
+- @store.setex "rabbit2", 1, @white_rabbit
+- @store.get("rabbit2").must_equal(@white_rabbit)
+- sleep 2
+- @store.get("rabbit2").must_be_nil
+- end
+-
+- it "doesn't unmarshal on multi get" do
+- @store.set "rabbit2", @white_rabbit
+- rabbit, rabbit2 = @store.mget "rabbit", "rabbit2"
+- rabbit.must_equal(@rabbit)
+- rabbit2.must_equal(@white_rabbit)
+- end
+-
+- if RUBY_VERSION.match /1\.9/
+- it "doesn't unmarshal on multi get if raw option is true" do
+- @store.set "rabbit2", @white_rabbit
+- rabbit, rabbit2 = @store.mget "rabbit", "rabbit2", :raw => true
+- rabbit.must_equal("\x04\bU:\x0FOpenStruct{\x06:\tnameI\"\nbunny\x06:\x06EF")
+- rabbit2.must_equal("\x04\bU:\x0FOpenStruct{\x06:\ncolorI\"\nwhite\x06:\x06EF")
+- end
+- else
+- it "doesn't unmarshal on multi get if raw option is true" do
+- @store.set "rabbit2", @white_rabbit
+- rabbit, rabbit2 = @store.mget "rabbit", "rabbit2", :raw => true
+- rabbit.must_include("\x04\bU:\x0FOpenStruct{\x06:\tname")
+- rabbit2.must_include("\x04\bU:\x0FOpenStruct{\x06:\ncolor")
+- end
+- end
+-
+- describe "binary safety" do
+- it "marshals objects" do
+- utf8_key = [51339].pack("U*")
+- ascii_rabbit = OpenStruct.new(:name => [128].pack("C*"))
+-
+- @store.set(utf8_key, ascii_rabbit)
+- @store.get(utf8_key).must_equal(ascii_rabbit)
+- end
+-
+- it "gets and sets raw values" do
+- utf8_key = [51339].pack("U*")
+- ascii_string = [128].pack("C*")
+-
+- @store.set(utf8_key, ascii_string, :raw => true)
+- @store.get(utf8_key, :raw => true).bytes.to_a.must_equal(ascii_string.bytes.to_a)
+- end
+-
+- it "marshals objects on setnx" do
+- utf8_key = [51339].pack("U*")
+- ascii_rabbit = OpenStruct.new(:name => [128].pack("C*"))
+-
+- @store.del(utf8_key)
+- @store.setnx(utf8_key, ascii_rabbit)
+- @store.get(utf8_key).must_equal(ascii_rabbit)
+- end
+-
+- it "gets and sets raw values on setnx" do
+- utf8_key = [51339].pack("U*")
+- ascii_string = [128].pack("C*")
+-
+- @store.del(utf8_key)
+- @store.setnx(utf8_key, ascii_string, :raw => true)
+- @store.get(utf8_key, :raw => true).bytes.to_a.must_equal(ascii_string.bytes.to_a)
+- end
+- end if defined?(Encoding)
+-end
+--- /dev/null
++++ b/test/redis/store/serialization_test.rb
+@@ -0,0 +1,128 @@
++require 'test_helper'
++
++describe "Redis::Serialization" do
++ def setup
++ @store = Redis::Store.new serializer: Marshal
++ @rabbit = OpenStruct.new :name => "bunny"
++ @white_rabbit = OpenStruct.new :color => "white"
++ @store.set "rabbit", @rabbit
++ @store.del "rabbit2"
++ end
++
++ def teardown
++ @store.flushdb
++ @store.quit
++ end
++
++ it "unmarshals on get" do
++ @store.get("rabbit").must_equal(@rabbit)
++ end
++
++ it "marshals on set" do
++ @store.set "rabbit", @white_rabbit
++ @store.get("rabbit").must_equal(@white_rabbit)
++ end
++
++ if RUBY_VERSION.match /1\.9/
++ it "doesn't unmarshal on get if raw option is true" do
++ @store.get("rabbit", :raw => true).must_equal("\x04\bU:\x0FOpenStruct{\x06:\tnameI\"\nbunny\x06:\x06EF")
++ end
++ else
++ it "doesn't unmarshal on get if raw option is true" do
++ @store.get("rabbit", :raw => true).must_include("\x04\bU:\x0FOpenStruct{\x06:\tname")
++ end
++ end
++
++ it "doesn't marshal set if raw option is true" do
++ @store.set "rabbit", @white_rabbit, :raw => true
++ @store.get("rabbit", :raw => true).must_equal(%(#<OpenStruct color="white">))
++ end
++
++ it "doesn't unmarshal if get returns an empty string" do
++ @store.set "empty_string", ""
++ @store.get("empty_string").must_equal("")
++ # TODO use a meaningful Exception
++ # lambda { @store.get("empty_string").must_equal("") }.wont_raise Exception
++ end
++
++ it "doesn't set an object if already exist" do
++ @store.setnx "rabbit", @white_rabbit
++ @store.get("rabbit").must_equal(@rabbit)
++ end
++
++ it "marshals on set unless exists" do
++ @store.setnx "rabbit2", @white_rabbit
++ @store.get("rabbit2").must_equal(@white_rabbit)
++ end
++
++ it "doesn't marshal on set unless exists if raw option is true" do
++ @store.setnx "rabbit2", @white_rabbit, :raw => true
++ @store.get("rabbit2", :raw => true).must_equal(%(#<OpenStruct color="white">))
++ end
++
++ it "marshals on set expire" do
++ @store.setex "rabbit2", 1, @white_rabbit
++ @store.get("rabbit2").must_equal(@white_rabbit)
++ sleep 2
++ @store.get("rabbit2").must_be_nil
++ end
++
++ it "doesn't unmarshal on multi get" do
++ @store.set "rabbit2", @white_rabbit
++ rabbit, rabbit2 = @store.mget "rabbit", "rabbit2"
++ rabbit.must_equal(@rabbit)
++ rabbit2.must_equal(@white_rabbit)
++ end
++
++ if RUBY_VERSION.match /1\.9/
++ it "doesn't unmarshal on multi get if raw option is true" do
++ @store.set "rabbit2", @white_rabbit
++ rabbit, rabbit2 = @store.mget "rabbit", "rabbit2", :raw => true
++ rabbit.must_equal("\x04\bU:\x0FOpenStruct{\x06:\tnameI\"\nbunny\x06:\x06EF")
++ rabbit2.must_equal("\x04\bU:\x0FOpenStruct{\x06:\ncolorI\"\nwhite\x06:\x06EF")
++ end
++ else
++ it "doesn't unmarshal on multi get if raw option is true" do
++ @store.set "rabbit2", @white_rabbit
++ rabbit, rabbit2 = @store.mget "rabbit", "rabbit2", :raw => true
++ rabbit.must_include("\x04\bU:\x0FOpenStruct{\x06:\tname")
++ rabbit2.must_include("\x04\bU:\x0FOpenStruct{\x06:\ncolor")
++ end
++ end
++
++ describe "binary safety" do
++ it "marshals objects" do
++ utf8_key = [51339].pack("U*")
++ ascii_rabbit = OpenStruct.new(:name => [128].pack("C*"))
++
++ @store.set(utf8_key, ascii_rabbit)
++ @store.get(utf8_key).must_equal(ascii_rabbit)
++ end
++
++ it "gets and sets raw values" do
++ utf8_key = [51339].pack("U*")
++ ascii_string = [128].pack("C*")
++
++ @store.set(utf8_key, ascii_string, :raw => true)
++ @store.get(utf8_key, :raw => true).bytes.to_a.must_equal(ascii_string.bytes.to_a)
++ end
++
++ it "marshals objects on setnx" do
++ utf8_key = [51339].pack("U*")
++ ascii_rabbit = OpenStruct.new(:name => [128].pack("C*"))
++
++ @store.del(utf8_key)
++ @store.setnx(utf8_key, ascii_rabbit)
++ @store.get(utf8_key).must_equal(ascii_rabbit)
++ end
++
++ it "gets and sets raw values on setnx" do
++ utf8_key = [51339].pack("U*")
++ ascii_string = [128].pack("C*")
++
++ @store.del(utf8_key)
++ @store.setnx(utf8_key, ascii_string, :raw => true)
++ @store.get(utf8_key, :raw => true).bytes.to_a.must_equal(ascii_string.bytes.to_a)
++ end
++ end if defined?(Encoding)
++end
+--- a/lib/redis/store/factory.rb
++++ b/lib/redis/store/factory.rb
+@@ -50,6 +50,14 @@
+ if options.key?(:key_prefix) && !options.key?(:namespace)
+ options[:namespace] = options.delete(:key_prefix) # RailsSessionStore
+ end
++ options[:raw] = case
++ when options.key?(:serializer)
++ options[:serializer].nil?
++ when options.key?(:marshalling)
++ !options[:marshalling]
++ else
++ false
++ end
+ options
+ end
+
+--- a/lib/redis/store/namespace.rb
++++ b/lib/redis/store/namespace.rb
+@@ -44,8 +44,8 @@
+ def mget(*keys)
+ options = keys.pop if keys.last.is_a? Hash
+ if keys.any?
+- # Marshalling gets extended before Namespace does, so we need to pass options further
+- if singleton_class.ancestors.include? Marshalling
++ # Serialization gets extended before Namespace does, so we need to pass options further
++ if singleton_class.ancestors.include? Serialization
+ super *keys.map {|key| interpolate(key) }, options
+ else
+ super *keys.map {|key| interpolate(key) }
diff -Nru ruby-redis-store-1.1.6/debian/patches/series ruby-redis-store-1.1.6/debian/patches/series
--- ruby-redis-store-1.1.6/debian/patches/series 2015-07-28 13:51:01.000000000 +0200
+++ ruby-redis-store-1.1.6/debian/patches/series 2017-12-01 17:21:20.000000000 +0100
@@ -1 +1,2 @@
Bundler
+CVE-2017-1000248.patch
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-ruby-extras-maintainers/attachments/20171201/fb6230ae/attachment-0001.sig>
More information about the Pkg-ruby-extras-maintainers
mailing list