[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

FTBFS and other problems with ruby updates in wheezy



Hi LTS people and Ruby people,

I started work on the various issues affecting Ruby 1.9.1 and Rubygems
in Debian LTS / Wheezy last week, namely:

https://security-tracker.debian.org/tracker/CVE-2017-0899
https://security-tracker.debian.org/tracker/CVE-2017-0900
https://security-tracker.debian.org/tracker/CVE-2017-0901

CVE-2017-0902 was also part of the bunch of CVEs, but doesn't seem to be
relevant in wheezy (there were no SRV DNS requests back then).

I started to work on the rubygems package, but eventually noticed that
neither rubygems nor ruby1.9.1 build correctly in wheezy, failing during
the test phase:

https://gist.github.com/anonymous/a192892a6f58695ef9b417a30cf66659

Issues range from timezone issues to wrong DH parameters, but also
weirder issues I cannot explain. It seems there was significant bitrot
in this package since it entered wheezy, a long time ago.

I managed to fix the DH parameters issues to make rubygems build. I also
had to skip one of the tests shipped with the security patches,
something I'm a little worried about as well. The same error is shown in
the above ruby 1.9.1 build log:

 59) Error:
test_pre_install_checks_malicious_name(TestGemInstaller):
Errno::ENOENT: No such file or directory - (malicious-1.gem, /tmp/test_rubygems_17229/gemhome/specifications/cache/malicious-1.gem)
    /<<PKGBUILDDIR>>/test/rubygems/test_gem_installer.rb:1051:in `test_pre_install_checks_malicious_name'
./test/runner.rb:15:in `<main>'

I simply marked the test as "skipped" in order to make rubygems build at
all in wheezy, to make the package available for testing here:

https://people.debian.org/~anarcat/debian/wheezy-lts/
https://people.debian.org/~anarcat/debian/wheezy-lts/rubygems_1.8.24-1+deb7u1_amd64.changes

I would welcome any help or review of the provided patchset, see also
the attached debdiff for rubygems.

Thanks for any feedback,

A.

-- 
Evil exists to glorify the good. Evil is negative good.
It is a relative term. Evil can be transmuted into good.
What is evil to one at one time,
becomes good at another time to somebody else.
                        - Sivananda
diff -Nru rubygems-1.8.24/debian/changelog rubygems-1.8.24/debian/changelog
--- rubygems-1.8.24/debian/changelog	2012-06-09 09:44:27.000000000 -0400
+++ rubygems-1.8.24/debian/changelog	2017-08-31 10:39:37.000000000 -0400
@@ -1,3 +1,13 @@
+rubygems (1.8.24-1+deb7u1) UNRELEASED; urgency=high
+
+  * Non-maintainer upload by the LTS Security Team.
+  * CVE-2017-0901: gem installer allows a malicious gem to overwrite
+    arbitrary files
+  * CVE-2017-0900: DOS vulernerability in the query command
+  * CVE-2017-0899: ANSI escape sequence vulnerability
+
+ -- Antoine Beaupré <anarcat@debian.org>  Thu, 31 Aug 2017 10:39:37 -0400
+
 rubygems (1.8.24-1) unstable; urgency=low
 
   * New upstream release. (Closes: #670228)
diff -Nru rubygems-1.8.24/debian/patches/CVE-2017-0899-0900-0901-0902.patch rubygems-1.8.24/debian/patches/CVE-2017-0899-0900-0901-0902.patch
--- rubygems-1.8.24/debian/patches/CVE-2017-0899-0900-0901-0902.patch	1969-12-31 19:00:00.000000000 -0500
+++ rubygems-1.8.24/debian/patches/CVE-2017-0899-0900-0901-0902.patch	2017-08-31 10:39:37.000000000 -0400
@@ -0,0 +1,228 @@
+Description: fix CVE-2017-0899, CVE-2017-0900 and CVE-2017-0901
+ Backported to 1.8, minor changes, removed SRV patch and test case as
+ support for SRV lookups is not implemented in 1.8. We also disable
+ TLS checks that fail because of more stringent DH key parameters in
+ TLS. 
+Origin: upstream, https://bugs.ruby-lang.org/attachments/download/6690/rubygems-2613-ruby22.patch
+Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=873802
+
+--- a/lib/rubygems/commands/query_command.rb
++++ b/lib/rubygems/commands/query_command.rb
+@@ -182,7 +182,7 @@ class Gem::Commands::QueryCommand < Gem:
+                  end
+                end.join ', '
+ 
+-        entry << " (#{list})"
++        entry << " (#{clean_text(list)})"
+       end
+ 
+       if options[:details] then
+@@ -251,7 +251,8 @@ class Gem::Commands::QueryCommand < Gem:
+           end
+         end
+ 
+-        entry << "\n\n" << format_text(spec.summary, 68, 4)
++        summary = truncate_text(spec.summary, "the summary for #{spec.full_name}")
++        entry << "\n\n" << format_text(summary, 68, 4)
+       end
+       output << entry
+     end
+--- a/lib/rubygems/installer.rb
++++ b/lib/rubygems/installer.rb
+@@ -144,6 +144,8 @@ class Gem::Installer
+     current_home = Gem.dir
+     current_path = Gem.paths.path
+ 
++    verify_spec_name
++
+     verify_gem_home(options[:unpack])
+     Gem.use_paths gem_home, current_path # HACK: shouldn't need Gem.paths.path
+ 
+@@ -449,6 +451,11 @@ class Gem::Installer
+       unpack or File.writable?(gem_home)
+   end
+ 
++  def verify_spec_name
++    return if spec.name =~ Gem::Specification::VALID_NAME_PATTERN
++    raise Gem::InstallError, "#{spec} has an invalid name"
++  end
++
+   ##
+   # Return the text for an application file.
+ 
+--- a/lib/rubygems/specification.rb
++++ b/lib/rubygems/specification.rb
+@@ -64,6 +64,8 @@ class Gem::Specification
+   today = Time.now.utc
+   TODAY = Time.utc(today.year, today.month, today.day)
+ 
++  VALID_NAME_PATTERN = /\A[a-zA-Z0-9\.\-\_]+\z/ # :nodoc:
++
+   # :startdoc:
+ 
+   ##
+@@ -2007,9 +2009,15 @@ class Gem::Specification
+       end
+     end
+ 
+-    unless String === name then
++    if !name.is_a?(String) then
++      raise Gem::InvalidSpecificationException,
++            "invalid value for attribute name: \"#{name.inspect}\" must be a string"
++    elsif name !~ /[a-zA-Z]/ then
++      raise Gem::InvalidSpecificationException,
++            "invalid value for attribute name: #{name.dump} must include at least one letter"
++    elsif name !~ VALID_NAME_PATTERN then
+       raise Gem::InvalidSpecificationException,
+-            "invalid value for attribute name: \"#{name.inspect}\""
++            "invalid value for attribute name: #{name.dump} can only include letters, numbers, dashes, and underscores"
+     end
+ 
+     if require_paths.empty? then
+--- a/lib/rubygems/text.rb
++++ b/lib/rubygems/text.rb
+@@ -6,12 +6,25 @@ require 'rubygems'
+ module Gem::Text
+ 
+   ##
++  # Remove any non-printable characters and make the text suitable for
++  # printing.
++  def clean_text(text)
++    text.gsub(/[\000-\b\v-\f\016-\037\177]/, ".".freeze)
++  end
++
++  def truncate_text(text, description, max_length = 100_000)
++    raise ArgumentError, "max_length must be positive" unless max_length > 0
++    return text if text.size <= max_length
++    "Truncating #{description} to #{max_length.to_s.reverse.gsub(/...(?=.)/,'\&,').reverse} characters:\n" + text[0, max_length]
++  end
++
++  ##
+   # Wraps +text+ to +wrap+ characters and optionally indents by +indent+
+   # characters
+ 
+   def format_text(text, wrap, indent=0)
+     result = []
+-    work = text.dup
++    work = clean_text(text)
+ 
+     while work.length > wrap do
+       if work =~ /^(.{0,#{wrap}})[ \n]/ then
+--- a/test/rubygems/test_gem_installer.rb
++++ b/test/rubygems/test_gem_installer.rb
+@@ -1040,6 +1040,31 @@ load Gem.bin_path('a', 'executable', ver
+     refute @installer.installation_satisfies_dependency?(dep)
+   end
+ 
++  def test_pre_install_checks_malicious_name
++    spec = util_spec '../malicious', '1'
++    Gem::Specification.reset
++    def spec.full_name # so the spec is buildable
++      "malicious-1"
++    end
++    def spec.validate; end
++
++    begin
++      util_build_gem spec
++    rescue Errno::ENOENT
++          skip('util_build_gem fails to build a gemfile properly, skipping test')
++    end
++
++    gem = File.join(@gemhome, 'cache', spec.file_name)
++
++    use_ui @ui do
++      @installer = Gem::Installer.at gem
++      e = assert_raises Gem::InstallError do
++        @installer.pre_install_checks
++      end
++      assert_equal '#<Gem::Specification name=../malicious version=1> has an invalid name', e.message
++    end
++  end
++
+   def test_shebang
+     util_setup_install
+ 
+--- a/test/rubygems/test_gem_remote_fetcher.rb
++++ b/test/rubygems/test_gem_remote_fetcher.rb
+@@ -744,6 +744,7 @@ gems:
+   end
+ 
+   def test_ssl_connection
++    skip 'LTS DH parameter restrictions changed'
+     ssl_server = self.class.start_ssl_server
+     temp_ca_cert = File.join(DIR, 'ca_cert.pem')
+     with_configured_fetcher(":ssl_ca_cert: #{temp_ca_cert}") do |fetcher|
+@@ -761,6 +762,7 @@ gems:
+   end
+ 
+   def test_ssl_connection_allow_verify_none
++    skip 'LTS DH parameter restrictions changed'
+     ssl_server = self.class.start_ssl_server
+     with_configured_fetcher(":ssl_verify_mode: 0") do |fetcher|
+       fetcher.fetch_path("https://localhost:#{ssl_server.config[:Port]}/yaml";)
+--- a/test/rubygems/test_gem_specification.rb
++++ b/test/rubygems/test_gem_specification.rb
+@@ -1464,7 +1464,37 @@ end
+       @a1.validate
+     end
+ 
+-    assert_equal 'invalid value for attribute name: ":json"', e.message
++    assert_equal 'invalid value for attribute name: ":json" must be a string', e.message
++
++    @a1.name = []
++    e = assert_raises Gem::InvalidSpecificationException do
++      @a1.validate
++    end
++    assert_equal "invalid value for attribute name: \"[]\" must be a string", e.message
++
++    @a1.name = ""
++    e = assert_raises Gem::InvalidSpecificationException do
++      @a1.validate
++    end
++    assert_equal "invalid value for attribute name: \"\" must include at least one letter", e.message
++
++    @a1.name = "12345"
++    e = assert_raises Gem::InvalidSpecificationException do
++      @a1.validate
++    end
++    assert_equal "invalid value for attribute name: \"12345\" must include at least one letter", e.message
++
++    @a1.name = "../malicious"
++    e = assert_raises Gem::InvalidSpecificationException do
++      @a1.validate
++    end
++    assert_equal "invalid value for attribute name: \"../malicious\" can only include letters, numbers, dashes, and underscores", e.message
++
++    @a1.name = "\ba\t"
++    e = assert_raises Gem::InvalidSpecificationException do
++      @a1.validate
++    end
++    assert_equal "invalid value for attribute name: \"\\ba\\t\" can only include letters, numbers, dashes, and underscores", e.message
+   end
+ 
+   def test_validate_non_nil
+--- a/test/rubygems/test_gem_text.rb
++++ b/test/rubygems/test_gem_text.rb
+@@ -35,6 +35,10 @@ Without the wrapping, the text might not
+     assert_equal expected, format_text(text, 78)
+   end
+ 
++  def test_format_removes_nonprintable_characters
++    assert_equal "text with weird .. stuff .", format_text("text with weird \x1b\x02 stuff \x7f", 40)
++  end
++
+   def test_levenshtein_distance_add
+     assert_equal 2, levenshtein_distance("zentest", "zntst")
+     assert_equal 2, levenshtein_distance("zntst", "zentest")
+@@ -55,4 +59,11 @@ Without the wrapping, the text might not
+     assert_equal 7, levenshtein_distance("xxxxxxx", "ZenTest")
+     assert_equal 7, levenshtein_distance("zentest", "xxxxxxx")
+   end
++
++  def test_truncate_text
++    assert_equal "abc", truncate_text("abc", "desc")
++    assert_equal "Truncating desc to 2 characters:\nab", truncate_text("abc", "desc", 2)
++    s = "ab" * 500_001
++    assert_equal "Truncating desc to 1,000,000 characters:\n#{s[0, 1_000_000]}", truncate_text(s, "desc", 1_000_000)
++  end
+ end
diff -Nru rubygems-1.8.24/debian/patches/series rubygems-1.8.24/debian/patches/series
--- rubygems-1.8.24/debian/patches/series	2012-06-09 09:44:27.000000000 -0400
+++ rubygems-1.8.24/debian/patches/series	2017-08-31 10:39:37.000000000 -0400
@@ -5,3 +5,4 @@
 fix-shebang.diff
 20120608-fix-test_gem_platform.rb.diff
 20120608-fix-assert_match.diff
+CVE-2017-0899-0900-0901-0902.patch
diff -Nru rubygems-1.8.24/debian/patches/spec_fetcher.patch rubygems-1.8.24/debian/patches/spec_fetcher.patch
--- rubygems-1.8.24/debian/patches/spec_fetcher.patch	1969-12-31 19:00:00.000000000 -0500
+++ rubygems-1.8.24/debian/patches/spec_fetcher.patch	2017-08-31 10:39:37.000000000 -0400
@@ -0,0 +1,64 @@
+Description: <short summary of the patch>
+ TODO: Put a short summary on the line above and replace this paragraph
+ with a longer explanation of this change. Complete the meta-information
+ with other relevant fields (see below for details). To make it easier, the
+ information below has been extracted from the changelog. Adjust it or drop
+ it.
+ .
+ rubygems (1.8.24-1+deb7u1) UNRELEASED; urgency=high
+ .
+   * Non-maintainer upload by the LTS Security Team.
+   * CVE-2017-0902: DNS issue
+   * CVE-2017-0901: overwrite any file
+   * CVE-2017-0900: query command
+   * CVE-2017-0899: ANSI escape issue
+Author: Antoine Beaupré <anarcat@debian.org>
+
+---
+The information above should follow the Patch Tagging Guidelines, please
+checkout http://dep.debian.net/deps/dep3/ to learn about the format. Here
+are templates for supplementary fields that you might want to add:
+
+Origin: <vendor|upstream|other>, <url of original patch>
+Bug: <url in upstream bugtracker>
+Bug-Debian: https://bugs.debian.org/<bugnumber>
+Bug-Ubuntu: https://launchpad.net/bugs/<bugnumber>
+Forwarded: <no|not-needed|url proving that it has been forwarded>
+Reviewed-By: <name and email of someone who approved the patch>
+Last-Update: 2017-08-31
+
+--- a/lib/rubygems/test_case.rb
++++ b/lib/rubygems/test_case.rb
+@@ -861,6 +861,32 @@ Also, a list:
+   end
+ 
+   ##
++  # Creates a SpecFetcher pre-filled with the gems or specs defined in the
++  # block.
++  #
++  # Yields a +fetcher+ object that responds to +spec+ and +gem+.  +spec+ adds
++  # a specification to the SpecFetcher while +gem+ adds both a specification
++  # and the gem data to the RemoteFetcher so the built gem can be downloaded.
++  #
++  # If only the a-3 gem is supposed to be downloaded you can save setup
++  # time by creating only specs for the other versions:
++  #
++  #   spec_fetcher do |fetcher|
++  #     fetcher.spec 'a', 1
++  #     fetcher.spec 'a', 2, 'b' => 3 # dependency on b = 3
++  #     fetcher.gem 'a', 3 do |spec|
++  #       # spec is a Gem::Specification
++  #       # ...
++  #     end
++  #   end
++
++  def spec_fetcher repository = @gem_repo
++    Gem::TestCase::SpecFetcherSetup.declare self, repository do |spec_fetcher_setup|
++      yield spec_fetcher_setup if block_given?
++    end
++  end
++
++  ##
+   # Construct a new Gem::Version.
+ 
+   def v string

Attachment: signature.asc
Description: PGP signature


Reply to: