Package: release.debian.org
Severity: normal
Tags: stretch
User: release.debian....@packages.debian.org
Usertags: pu


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).

It was proposed by the security team to fx it with a Stretch update
instead of a security upload.

Thanks

Cédric

-- System Information:
Debian Release: buster/sid
  APT prefers unstable-debug
  APT policy: (500, 'unstable-debug'), (500, 'unstable'), (500, 'testing'), (1, 
'experimental')
Architecture: amd64 (x86_64)

Kernel: Linux 4.14.0-3-amd64 (SMP w/4 CPU cores)
Locale: LANG=fr_FR.UTF-8, LC_CTYPE=fr_FR.UTF-8 (charmap=UTF-8), LANGUAGE= 
(charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled
diff --git a/debian/changelog b/debian/changelog
index d28f11f..83455a6 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,11 @@
+ruby-redis-store (1.1.6-1+deb9u1) stretch; 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 <bou...@debian.org>  Fri, 01 Dec 2017 17:22:29 +0100
+
 ruby-redis-store (1.1.6-1) unstable; urgency=medium
 
   * Upstream update
diff --git a/debian/patches/CVE-2017-1000248.patch 
b/debian/patches/CVE-2017-1000248.patch
new file mode 100644
index 0000000..44c91de
--- /dev/null
+++ b/debian/patches/CVE-2017-1000248.patch
@@ -0,0 +1,551 @@
+Description: Replace marshalling with pluggable serializers
+Author: Tom Scott <tsc...@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 --git a/debian/patches/series b/debian/patches/series
index a51ad82..beee078 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1 +1,2 @@
 Bundler
+CVE-2017-1000248.patch

Reply via email to