guix-devel
[Top][All Lists]
Advanced

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

[PATCH 1/1] gnu: ruby-minitar: Fix an arbitrary file overwrite bug.


From: Leo Famulari
Subject: [PATCH 1/1] gnu: ruby-minitar: Fix an arbitrary file overwrite bug.
Date: Tue, 24 Jan 2017 14:57:06 -0500

* gnu/packages/patches/minitar-fix-arbitrary-file-overwrite.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.
* gnu/packages/ruby.scm (ruby-minitar)[source]: Use it.
---
 gnu/local.mk                                       |   1 +
 .../minitar-fix-arbitrary-file-overwrite.patch     | 253 +++++++++++++++++++++
 gnu/packages/ruby.scm                              |   1 +
 3 files changed, 255 insertions(+)
 create mode 100644 
gnu/packages/patches/minitar-fix-arbitrary-file-overwrite.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index 3963b97b7..14aa56a75 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -865,6 +865,7 @@ dist_patch_DATA =                                           
\
   %D%/packages/patches/rpm-CVE-2014-8118.patch                 \
   %D%/packages/patches/rsem-makefile.patch                     \
   %D%/packages/patches/ruby-concurrent-ignore-broken-test.patch        \
+  %D%/packages/patches/minitar-fix-arbitrary-file-overwrite.patch      \
   %D%/packages/patches/ruby-puma-ignore-broken-test.patch       \
   %D%/packages/patches/ruby-rack-ignore-failing-test.patch      \
   %D%/packages/patches/ruby-tzinfo-data-ignore-broken-test.patch\
diff --git a/gnu/packages/patches/minitar-fix-arbitrary-file-overwrite.patch 
b/gnu/packages/patches/minitar-fix-arbitrary-file-overwrite.patch
new file mode 100644
index 000000000..5d1836a09
--- /dev/null
+++ b/gnu/packages/patches/minitar-fix-arbitrary-file-overwrite.patch
@@ -0,0 +1,253 @@
+Fix a bug allowing arbitrary file overwrite during archive extraction
+via '..' directory traversal:
+
+http://seclists.org/oss-sec/2017/q1/178
+https://github.com/halostatue/minitar/issues/16
+
+Patch copied from upstream source repository:
+
+https://github.com/halostatue/minitar/commit/e25205ecbb6277ae8a3df1e6a306d7ed4458b6e4
+
+From e25205ecbb6277ae8a3df1e6a306d7ed4458b6e4 Mon Sep 17 00:00:00 2001
+From: Austin Ziegler <address@hidden>
+Date: Sun, 13 Nov 2016 23:25:21 -0500
+Subject: [PATCH] Resolve relative path vulnerability
+
+Fixes #16
+---
+ History.md                       | 37 +++++++++++++++++++++++++------------
+ lib/archive/tar/minitar.rb       | 11 ++++++++---
+ lib/archive/tar/minitar/input.rb | 34 +++++++++++++++++++++++++++-------
+ minitar.gemspec                  |  2 +-
+ test/test_tar_input.rb           | 32 ++++++++++++++++++++++++++++++++
+ 5 files changed, 93 insertions(+), 23 deletions(-)
+
+diff --git a/History.md b/History.md
+index 6fd68cc..adfd992 100644
+--- a/History.md
++++ b/History.md
+@@ -8,6 +8,14 @@
+         `archive-tar-minitar` will install both `minitar` and `minitar-cli`, 
at
+         least until version 1.0.)
+ 
++    *   Minitar extraction before 0.6 traverses directories if the tarball
++        includes a relative directory reference, as reported in [#16][] by
++        @ecneladis. This has been disallowed entirely and will throw a
++        SecureRelativePathError when found. Additionally, if the final
++        destination of an entry is an already-existing symbolic link, the
++        existing symbolic link will be removed and the file will be written
++        correctly (on platforms that support symblic links).
++
+ *   Enhancements:
+ 
+     *   Licence change. After speaking with Mauricio Fernández, we have 
changed
+@@ -51,18 +59,16 @@
+ 
+ *   Bugs:
+ 
+-    *   Fix [#2](https://github.com/halostatue/minitar/issues/2) to handle IO
+-        streams that are not seekable, such as pipes, STDIN, or STDOUT.
+-    *   Fix [#3](https://github.com/halostatue/minitar/issues/3) to make the
+-        test timezone resilient.
+-    *   Fix [#4](https://github.com/halostatue/minitar/issues/4) for 
supporting
+-        the reading of tar files with filenames in the GNU long filename
+-        extension format. Ported from @atoulme’s fork, originally provided by
+-        Curtis Sampson.
+-    *   Fix [#6](https://github.com/halostatue/minitar/issues/6) by making it
+-        raise the correct error for a long filename with no path components.
+-    *   Fix [#14](https://github.com/halostatue/minitar/pull/6) provided by
+-        @kzys should fix Windows detection issues.
++    *   Fix [#2][] to handle IO streams that are not seekable, such as pipes,
++        STDIN, or STDOUT.
++    *   Fix [#3][] to make the test timezone resilient.
++    *   Fix [#4][] for supporting the reading of tar files with filenames in
++        the GNU long filename extension format. Ported from @atoulme’s fork,
++        originally provided by Curtis Sampson.
++    *   Fix [#6][] by making it raise the correct error for a long filename
++        with no path components.
++    *   Fix [#14][] provided by @kzys should fix Windows detection issues.
++    *   Fix [#16][] as specified above.
+ 
+ *   Development:
+ 
+@@ -83,3 +89,10 @@
+ 
+ * Initial release. Does files and directories. Command does create, extract,
+   and list.
++
++[#2]: https://github.com/halostatue/minitar/issues/2
++[#3]: https://github.com/halostatue/minitar/issues/3
++[#4]: https://github.com/halostatue/minitar/issues/4
++[#6]: https://github.com/halostatue/minitar/issues/6
++[#14]: https://github.com/halostatue/minitar/issues/14
++[#16]: https://github.com/halostatue/minitar/issues/16
+diff --git a/lib/archive/tar/minitar.rb b/lib/archive/tar/minitar.rb
+index b69a3a1..b38bc7a 100644
+--- a/lib/archive/tar/minitar.rb
++++ b/lib/archive/tar/minitar.rb
+@@ -73,17 +73,22 @@ def modules
+ module Archive::Tar::Minitar
+   VERSION = '0.6' # :nodoc:
+ 
++  # The base class for any minitar error.
++  Error = Class.new(StandardError)
+   # Raised when a wrapped data stream class is not seekable.
+-  NonSeekableStream = Class.new(StandardError)
++  NonSeekableStream = Class.new(Error)
+   # The exception raised when operations are performed on a stream that has
+   # previously been closed.
+-  ClosedStream = Class.new(StandardError)
++  ClosedStream = Class.new(Error)
+   # The exception raised when a filename exceeds 256 bytes in length, the
+   # maximum supported by the standard Tar format.
+-  FileNameTooLong = Class.new(StandardError)
++  FileNameTooLong = Class.new(Error)
+   # The exception raised when a data stream ends before the amount of data
+   # expected in the archive's PosixHeader.
+   UnexpectedEOF = Class.new(StandardError)
++  # The exception raised when a file contains a relative path in secure mode
++  # (the default for this version).
++  SecureRelativePathError = Class.new(Error)
+ 
+   class << self
+     # Tests if +path+ refers to a directory. Fixes an apparently
+diff --git a/lib/archive/tar/minitar/input.rb 
b/lib/archive/tar/minitar/input.rb
+index 599948e..4ba27fe 100644
+--- a/lib/archive/tar/minitar/input.rb
++++ b/lib/archive/tar/minitar/input.rb
+@@ -97,10 +97,25 @@ def extract_entry(destdir, entry) # :yields action, name, 
stats:
+         :entry    => entry
+       }
+ 
++      # extract_entry is not vulnerable to prefix '/' vulnerabilities, but it
++      # is vulnerable to relative path directories. This code will break this
++      # vulnerability. For this version, we are breaking relative paths HARD 
by
++      # throwing an exception.
++      #
++      # Future versions may permit relative paths as long as the file does not
++      # leave +destdir+.
++      #
++      # However, squeeze consecutive '/' characters together.
++      full_name = entry.full_name.squeeze('/')
++
++      if full_name =~ /\.{2}(?:\/|\z)/
++        raise SecureRelativePathError, %q(Path contains '..')
++      end
++
+       if entry.directory?
+-        dest = File.join(destdir, entry.full_name)
++        dest = File.join(destdir, full_name)
+ 
+-        yield :dir, entry.full_name, stats if block_given?
++        yield :dir, full_name, stats if block_given?
+ 
+         if Archive::Tar::Minitar.dir?(dest)
+           begin
+@@ -109,6 +124,8 @@ def extract_entry(destdir, entry) # :yields action, name, 
stats:
+             nil
+           end
+         else
++          File.unlink(dest.chomp('/')) if File.symlink?(dest.chomp('/'))
++
+           FileUtils.mkdir_p(dest, :mode => entry.mode)
+           FileUtils.chmod(entry.mode, dest)
+         end
+@@ -117,13 +134,16 @@ def extract_entry(destdir, entry) # :yields action, 
name, stats:
+         fsync_dir(File.join(dest, ".."))
+         return
+       else # it's a file
+-        destdir = File.join(destdir, File.dirname(entry.full_name))
++        destdir = File.join(destdir, File.dirname(full_name))
+         FileUtils.mkdir_p(destdir, :mode => 0755)
+ 
+-        destfile = File.join(destdir, File.basename(entry.full_name))
++        destfile = File.join(destdir, File.basename(full_name))
++
++        File.unlink(destfile) if File.symlink?(destfile)
++
+         FileUtils.chmod(0600, destfile) rescue nil  # Errno::ENOENT
+ 
+-        yield :file_start, entry.full_name, stats if block_given?
++        yield :file_start, full_name, stats if block_given?
+ 
+         File.open(destfile, "wb", entry.mode) do |os|
+           loop do
+@@ -133,7 +153,7 @@ def extract_entry(destdir, entry) # :yields action, name, 
stats:
+             stats[:currinc] = os.write(data)
+             stats[:current] += stats[:currinc]
+ 
+-            yield :file_progress, entry.full_name, stats if block_given?
++            yield :file_progress, full_name, stats if block_given?
+           end
+           os.fsync
+         end
+@@ -142,7 +162,7 @@ def extract_entry(destdir, entry) # :yields action, name, 
stats:
+         fsync_dir(File.dirname(destfile))
+         fsync_dir(File.join(File.dirname(destfile), ".."))
+ 
+-        yield :file_done, entry.full_name, stats if block_given?
++        yield :file_done, full_name, stats if block_given?
+       end
+     end
+ 
+diff --git a/minitar.gemspec b/minitar.gemspec
+index f0674c5..344606b 100644
+--- a/minitar.gemspec
++++ b/minitar.gemspec
+@@ -8,7 +8,7 @@ Gem::Specification.new do |s|
+   s.required_rubygems_version = Gem::Requirement.new(">= 0") if s.respond_to? 
:required_rubygems_version=
+   s.require_paths = ["lib"]
+   s.authors = ["Austin Ziegler"]
+-  s.date = "2016-11-08"
++  s.date = "2016-11-14"
+   s.description = "The minitar library is a pure-Ruby library that provides 
the ability to deal\nwith POSIX tar(1) archive files.\n\nThis is release 0.6, 
\u{2026}\n\nminitar (previously called Archive::Tar::Minitar) is based heavily 
on code\noriginally written by Mauricio Julio Fern\u{e1}ndez Pradier for the 
rpa-base\nproject."
+   s.email = ["address@hidden"]
+   s.extra_rdoc_files = ["Code-of-Conduct.md", "Contributing.md", 
"History.md", "Licence.md", "Manifest.txt", "README.rdoc", "docs/bsdl.txt", 
"docs/ruby.txt"]
+diff --git a/test/test_tar_input.rb b/test/test_tar_input.rb
+index 87dc4da..5e46ffb 100644
+--- a/test/test_tar_input.rb
++++ b/test/test_tar_input.rb
+@@ -73,6 +73,7 @@ def test_each_works
+ 
+         outer += 1
+       end
++
+       assert_equal(2, outer)
+     end
+   end
+@@ -131,4 +132,35 @@ def test_extract_entry_works
+       assert_equal(2, outer_count)
+     end
+   end
++
++  def test_extract_entry_breaks_symlinks
++    return if Minitar.windows?
++
++    IO.write('data__/file4', '')
++    File.symlink('data__/file4', 'data__/file3')
++    File.symlink('data__/file4', 'data__/data')
++
++    Minitar.unpack(Zlib::GzipReader.new(StringIO.new(TEST_TGZ)), 'data__')
++    Minitar.unpack(Zlib::GzipReader.new(File.open('data__/data.tar.gz', 
'rb')),
++                   'data__')
++
++    refute File.symlink?('data__/file3')
++    refute File.symlink?('data__/data')
++  end
++
++  RELATIVE_DIRECTORY_TGZ = Base64.decode64 <<-EOS
++H4sICIIoKVgCA2JhZC1kaXIudGFyANPT0y8sTy0qqWSgHTAwMDAzMVEA0eZmpmDawAjChwEFQ2MDQyMg
++MDUzVDAwNDY0N2VQMGCgAygtLkksAjolEcjIzMOtDqgsLQ2/J0H+gNOjYBSMglEwyAEA2LchrwAGAAA=
++  EOS
++
++  def test_extract_entry_fails_with_relative_directory
++    reader = Zlib::GzipReader.new(StringIO.new(RELATIVE_DIRECTORY_TGZ))
++    Minitar::Input.open(reader) do |stream|
++      stream.each do |entry|
++        assert_raises Archive::Tar::Minitar::SecureRelativePathError do
++          stream.extract_entry("data__", entry)
++        end
++      end
++    end
++  end
+ end
diff --git a/gnu/packages/ruby.scm b/gnu/packages/ruby.scm
index 0f1ecd29d..e2a7a88f3 100644
--- a/gnu/packages/ruby.scm
+++ b/gnu/packages/ruby.scm
@@ -1866,6 +1866,7 @@ generation of complex SQL queries and is compatible with 
various RDBMSes.")
      (origin
        (method url-fetch)
        (uri (rubygems-uri "minitar" version))
+       (patches (search-patches "minitar-fix-arbitrary-file-overwrite.patch"))
        (sha256
         (base32
          "1vpdjfmdq1yc4i620frfp9af02ia435dnpj8ybsd7dc3rypkvbka"))))
-- 
2.11.0




reply via email to

[Prev in Thread] Current Thread [Next in Thread]