[DRE-maint] rails update
Salvatore Bonaccorso
carnil at debian.org
Tue Jun 30 21:38:42 BST 2020
Hi Sylvain, rails maintainers,
On Mon, Jun 29, 2020 at 01:06:49PM +0200, Sylvain Beucler wrote:
> Hi,
>
> On 25/06/2020 18:20, Sylvain Beucler wrote:
> > On 22/06/2020 13:23, Sylvain Beucler wrote:
> >> On 22/06/2020 11:56, Utkarsh Gupta wrote:
> >>> On Mon, Jun 22, 2020 at 3:11 PM Sylvain Beucler <beuc at beuc.net> wrote:
> >>>> Hmm, are you the only active maintainer for rails?
> >>>
> >>> There are 3 maintainers. CC'ed rails at p.d.o.
> >>> However, since you have already worked on preparing the fix for
> >>> Jessie, it's much easier on your part to do it for Stretch and Buster.
> >>> But that's volunteer work :)
> >>>
> >>> If you don't want to work, don't :)
> >>
> >> For rails at d.p.o's info, I explained at:
> >> https://lists.debian.org/debian-lts/2020/06/msg00063.html
> >> that I prepared the jessie (4.1.8) and stretch (4.2.7.1) updates at:
> >> https://www.beuc.net/tmp/debian-lts/rails/
> >>
> >> However the buster version (5.2.2.1) is affected by a different set of
> >> vulnerabilities, is much closer to bullseye (5.2.4.3), and apparently
> >> the update causes new issues.
> >>
> >> That's why I think it'd make more sense for the rails maintainers to
> >> backport the latest bullseye update.
> >>
> >> Let me know what you plan to do.
> >>
> >>>> Which security update broke what, exactly?
> >>>
> >>> The latest security update from 5.2.4.2 to 5.2.4.3, which contained
> >>> fixes for CVE-2020-816{2,4,5,6,7}.
> >>> JavaScript bundle generation for Activestorage didn't work w/o that
> >>> patch. We had to switch to node-babel7 for that.
> >>
> >> I updated
> >> https://wiki.debian.org/LTS/TestSuites/rails
> >> accordingly.
> >>
> >> The stretch updates passes this new test.
> >>
> >> (Though in this particular case it may have just been due to node-babel
> >> changes in unstable since March, e.g. babel7 is pulled through
> >> node-regenerator-transform.)
> >
> > Status update: jessie and stretch are affected by new important
> > CVE-2020-8163.
> > buster and above not affected.
> > Currently waiting for upstream's feedback on a second regression, then
> > I'll prepare an update for jessie & stretch.
>
> https://www.beuc.net/tmp/debian-lts/rails/ is updated.
>
> Upstream showed little care for 4.x and I don't expect further feedback,
> so I went ahead and backported:
> https://github.com/rails/rails/commit/d9ff835b99ff3c7567ccde9b1379b4deeabee32f
> to fix the regression, including tests.
>
> Rationale at:
> https://github.com/rails/rails/issues/39301#issuecomment-648885623
>
> Note: redmine/stretch (< 3.4) was not affected by the regression.
Attaching the debdiff for reference. The changes looks good to me, but
I defintively would like to see a second pair of eyes here from the
rails maintainers, in particular for CVE-2020-8163, Utkarsh?
There is no lost work, but if we want to release a rails update for
stretch (before it moves to LTS), we should try to get as well a rails
update beeing prepared for buster, Utkarsh you indicated lack of time
currently, any one other up from the rails maintainers?
Regards,
Salvatore
-------------- next part --------------
diff -Nru rails-4.2.7.1/debian/changelog rails-4.2.7.1/debian/changelog
--- rails-4.2.7.1/debian/changelog 2020-03-22 13:35:32.000000000 +0100
+++ rails-4.2.7.1/debian/changelog 2020-06-29 09:55:00.000000000 +0200
@@ -1,3 +1,14 @@
+rails (2:4.2.7.1-1+deb9u3) stretch-security; urgency=high
+
+ * Non-maintainer upload by the LTS Security Team.
+ * CVE-2020-8164: possible Strong Parameters Bypass in ActionPack
+ * CVE-2020-8165: potentially unintended unmarshalling of user-provided
+ objects in MemCacheStore
+ * CVE-2020-8163: potential remote code execution of user-provided
+ local names
+
+ -- Sylvain Beucler <beuc at debian.org> Mon, 29 Jun 2020 09:55:00 +0200
+
rails (2:4.2.7.1-1+deb9u2) stretch; urgency=high
* Team upload.
diff -Nru rails-4.2.7.1/debian/patches/CVE-2020-8163.patch rails-4.2.7.1/debian/patches/CVE-2020-8163.patch
--- rails-4.2.7.1/debian/patches/CVE-2020-8163.patch 1970-01-01 01:00:00.000000000 +0100
+++ rails-4.2.7.1/debian/patches/CVE-2020-8163.patch 2020-06-29 09:55:00.000000000 +0200
@@ -0,0 +1,96 @@
+Origin: https://github.com/rails/rails/commit/4c46a15e0a7815ca9e4cd7c7fda042eb8c1b7724
+Origin: https://github.com/rails/rails/commit/d9ff835b99ff3c7567ccde9b1379b4deeabee32f
+Last-Update: 2029-06-29
+Reviewed-by: Sylvain Beucler <beuc at debian.org>
+
+Index: rails-4.2.7.1/actionview/lib/action_view/template.rb
+===================================================================
+--- rails-4.2.7.1.orig/actionview/lib/action_view/template.rb
++++ rails-4.2.7.1/actionview/lib/action_view/template.rb
+@@ -312,8 +312,12 @@ module ActionView
+ end
+
+ def locals_code #:nodoc:
++ # Only locals with valid variable names get set directly. Others will
++ # still be available in local_assigns.
++ locals = @locals.to_set - Module::RUBY_RESERVED_WORDS
++ locals = locals.grep(/\A(?![A-Z0-9])(?:[[:alnum:]_]|[^\0-\177])+\z/)
+ # Double assign to suppress the dreaded 'assigned but unused variable' warning
+- @locals.each_with_object('') { |key, code| code << "#{key} = #{key} = local_assigns[:#{key}];" }
++ locals.each_with_object('') { |key, code| code << "#{key} = #{key} = local_assigns[:#{key}];" }
+ end
+
+ def method_name #:nodoc:
+Index: rails-4.2.7.1/actionview/test/fixtures/test/render_file_inspect_local_assigns.erb
+===================================================================
+--- /dev/null
++++ rails-4.2.7.1/actionview/test/fixtures/test/render_file_inspect_local_assigns.erb
+@@ -0,0 +1 @@
++<%= local_assigns.inspect.html_safe %>
+\ No newline at end of file
+Index: rails-4.2.7.1/actionview/test/fixtures/test/render_file_unicode_local.erb
+===================================================================
+--- /dev/null
++++ rails-4.2.7.1/actionview/test/fixtures/test/render_file_unicode_local.erb
+@@ -0,0 +1 @@
++<%= 🎃 %>
+\ No newline at end of file
+Index: rails-4.2.7.1/actionview/test/fixtures/test/render_file_with_ruby_keyword_locals.erb
+===================================================================
+--- /dev/null
++++ rails-4.2.7.1/actionview/test/fixtures/test/render_file_with_ruby_keyword_locals.erb
+@@ -0,0 +1 @@
++The class is <%= local_assigns[:class] %>
+\ No newline at end of file
+Index: rails-4.2.7.1/actionview/test/fixtures/test/test_template_with_delegation_reserved_keywords.erb
+===================================================================
+--- /dev/null
++++ rails-4.2.7.1/actionview/test/fixtures/test/test_template_with_delegation_reserved_keywords.erb
+@@ -0,0 +1 @@
++<%= _ %> <%= arg %> <%= args %> <%= block %>
+\ No newline at end of file
+Index: rails-4.2.7.1/actionview/test/template/compiled_templates_test.rb
+===================================================================
+--- rails-4.2.7.1.orig/actionview/test/template/compiled_templates_test.rb
++++ rails-4.2.7.1/actionview/test/template/compiled_templates_test.rb
+@@ -1,3 +1,4 @@
++# coding: utf-8
+ require 'abstract_unit'
+
+ class CompiledTemplatesTest < ActiveSupport::TestCase
+@@ -9,6 +10,35 @@ class CompiledTemplatesTest < ActiveSupp
+ assert_equal "This is nil: \n", render(:template => "test/nil_return")
+ end
+
++ def test_template_with_ruby_keyword_locals
++ assert_equal "The class is foo",
++ render(file: "test/render_file_with_ruby_keyword_locals", locals: { class: "foo" })
++ end
++
++ def test_template_with_invalid_identifier_locals
++ locals = {
++ foo: "bar",
++ Foo: "bar",
++ "d-a-s-h-e-s": "",
++ "white space": "",
++ }
++ assert_equal locals.inspect, render(file: "test/render_file_inspect_local_assigns", locals: locals)
++ end
++
++ def test_template_with_delegation_reserved_keywords
++ locals = {
++ _: "one",
++ arg: "two",
++ args: "three",
++ block: "four",
++ }
++ assert_equal "one two three four", render(file: "test/test_template_with_delegation_reserved_keywords", locals: locals)
++ end
++
++ def test_template_with_unicode_identifier
++ assert_equal "🎂", render(file: "test/render_file_unicode_local", locals: { 🎃: "🎂" })
++ end
++
+ def test_template_gets_recompiled_when_using_different_keys_in_local_assigns
+ assert_equal "one", render(:file => "test/render_file_with_locals_and_default")
+ assert_equal "two", render(:file => "test/render_file_with_locals_and_default", :locals => { :secret => "two" })
diff -Nru rails-4.2.7.1/debian/patches/CVE-2020-8164.patch rails-4.2.7.1/debian/patches/CVE-2020-8164.patch
--- rails-4.2.7.1/debian/patches/CVE-2020-8164.patch 1970-01-01 01:00:00.000000000 +0100
+++ rails-4.2.7.1/debian/patches/CVE-2020-8164.patch 2020-06-18 18:07:48.000000000 +0200
@@ -0,0 +1,48 @@
+Origin: https://github.com/rails/rails/commit/7a3ee4fea90b7555f8d09c6c05c15fe7ab5a06ec
+Last-Update: 2029-06-18
+Reviewed-by: Sylvain Beucler <beuc at debian.org>
+
+From 7a3ee4fea90b7555f8d09c6c05c15fe7ab5a06ec Mon Sep 17 00:00:00 2001
+From: Jack McCracken <jack.mccracken at shopify.com>
+Date: Wed, 13 May 2020 15:25:12 -0400
+Subject: [PATCH] Return self when calling #each, #each_pair, and #each_value
+ instead of the raw @parameters hash
+
+[CVE-2020-8164]
+---
+ .../lib/action_controller/metal/strong_parameters.rb | 2 ++
+ actionpack/test/controller/parameters/accessors_test.rb | 8 ++++++++
+ 2 files changed, 10 insertions(+)
+
+Index: rails-4.2.7.1/actionpack/lib/action_controller/metal/strong_parameters.rb
+===================================================================
+--- rails-4.2.7.1.orig/actionpack/lib/action_controller/metal/strong_parameters.rb
++++ rails-4.2.7.1/actionpack/lib/action_controller/metal/strong_parameters.rb
+@@ -400,6 +400,8 @@ module ActionController
+ else
+ super
+ end
++
++ self
+ end
+
+ # This method is here only to make sure that the returned object has the
+Index: rails-4.2.7.1/actionpack/test/controller/parameters/accessors_test.rb
+===================================================================
+--- rails-4.2.7.1.orig/actionpack/test/controller/parameters/accessors_test.rb
++++ rails-4.2.7.1/actionpack/test/controller/parameters/accessors_test.rb
+@@ -16,6 +16,14 @@ class ParametersAccessorsTest < ActiveSu
+ )
+ end
+
++ test "each returns self" do
++ assert_same @params, @params.each { |_| _ }
++ end
++
++ test "each_pair returns self" do
++ assert_same @params, @params.each_pair { |_| _ }
++ end
++
+ test "[] retains permitted status" do
+ @params.permit!
+ assert @params[:person].permitted?
diff -Nru rails-4.2.7.1/debian/patches/CVE-2020-8165.patch rails-4.2.7.1/debian/patches/CVE-2020-8165.patch
--- rails-4.2.7.1/debian/patches/CVE-2020-8165.patch 1970-01-01 01:00:00.000000000 +0100
+++ rails-4.2.7.1/debian/patches/CVE-2020-8165.patch 2020-06-18 18:13:17.000000000 +0200
@@ -0,0 +1,82 @@
+Origin: https://github.com/rails/rails/commit/f7e077f85e61fc0b7381963eda0ceb0e457546b5
+Last-Update: 2029-06-18
+Reviewed-by: Sylvain Beucler <beuc at debian.org>
+
+From f7e077f85e61fc0b7381963eda0ceb0e457546b5 Mon Sep 17 00:00:00 2001
+From: Dylan Thacker-Smith <Dylan.Smith at shopify.com>
+Date: Sat, 22 Sep 2018 17:57:58 -0400
+Subject: [PATCH] activesupport: Avoid Marshal.load on raw cache value in
+ MemCacheStore
+
+Dalli is already being used for marshalling, so we should also rely
+on it for unmarshalling. Since Dalli tags the cache value as marshalled
+it can avoid unmarshalling a raw string which might have come from
+an untrusted source.
+
+[CVE-2020-8165]
+---
+ .../lib/active_support/cache/mem_cache_store.rb | 14 ++------------
+ .../test/cache/stores/mem_cache_store_test.rb | 4 ++--
+ 2 files changed, 4 insertions(+), 14 deletions(-)
+
+Index: rails-4.2.7.1/activesupport/lib/active_support/cache/mem_cache_store.rb
+===================================================================
+--- rails-4.2.7.1.orig/activesupport/lib/active_support/cache/mem_cache_store.rb
++++ rails-4.2.7.1/activesupport/lib/active_support/cache/mem_cache_store.rb
+@@ -6,7 +6,6 @@ rescue LoadError => e
+ end
+
+ require 'digest/md5'
+-require 'active_support/core_ext/marshal'
+ require 'active_support/core_ext/array/extract_options'
+
+ module ActiveSupport
+@@ -163,9 +162,8 @@ module ActiveSupport
+ key
+ end
+
+- def deserialize_entry(raw_value)
+- if raw_value
+- entry = Marshal.load(raw_value) rescue raw_value
++ def deserialize_entry(entry)
++ if entry
+ entry.is_a?(Entry) ? entry : Entry.new(entry)
+ else
+ nil
+@@ -175,14 +173,6 @@ module ActiveSupport
+ # Provide support for raw values in the local cache strategy.
+ module LocalCacheWithRaw # :nodoc:
+ protected
+- def read_entry(key, options)
+- entry = super
+- if options[:raw] && local_cache && entry
+- entry = deserialize_entry(entry.value)
+- end
+- entry
+- end
+-
+ def write_entry(key, entry, options) # :nodoc:
+ retval = super
+ if options[:raw] && local_cache && retval
+Index: rails-4.2.7.1/activesupport/test/caching_test.rb
+===================================================================
+--- rails-4.2.7.1.orig/activesupport/test/caching_test.rb
++++ rails-4.2.7.1/activesupport/test/caching_test.rb
+@@ -928,7 +928,7 @@ class MemCacheStoreTest < ActiveSupport:
+ cache = ActiveSupport::Cache.lookup_store(:mem_cache_store, :raw => true)
+ cache.clear
+ cache.write("foo", Marshal.dump([]))
+- assert_equal [], cache.read("foo")
++ assert_equal Marshal.dump([]), cache.read("foo")
+ end
+
+ def test_local_cache_raw_values
+@@ -945,7 +945,7 @@ class MemCacheStoreTest < ActiveSupport:
+ cache.clear
+ cache.with_local_cache do
+ cache.write("foo", Marshal.dump([]))
+- assert_equal [], cache.read("foo")
++ assert_equal Marshal.dump([]), cache.read("foo")
+ end
+ end
+
diff -Nru rails-4.2.7.1/debian/patches/series rails-4.2.7.1/debian/patches/series
--- rails-4.2.7.1/debian/patches/series 2020-03-22 13:34:25.000000000 +0100
+++ rails-4.2.7.1/debian/patches/series 2020-06-29 09:50:22.000000000 +0200
@@ -5,3 +5,6 @@
006-CVE-2018-16476.patch
007-CVE-2019-5418_CVE-2019-5419.patch
CVE-2020-5267.patch
+CVE-2020-8164.patch
+CVE-2020-8165.patch
+CVE-2020-8163.patch
More information about the Pkg-ruby-extras-maintainers
mailing list