* [PATCH 1/1] gnu: ruby-minitar: Fix an arbitrary file overwrite bug.
2017-01-24 19:57 [PATCH 0/1] Help wanted fixing a ruby-minitar bug Leo Famulari
@ 2017-01-24 19:57 ` Leo Famulari
2017-01-24 21:41 ` Ludovic Courtès
2017-01-24 21:40 ` [PATCH 0/1] Help wanted fixing a ruby-minitar bug Ludovic Courtès
1 sibling, 1 reply; 8+ messages in thread
From: Leo Famulari @ 2017-01-24 19:57 UTC (permalink / raw)
To: guix-devel
* 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 <austin@zieglers.ca>
+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 = ["halostatue@gmail.com"]
+ 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
^ permalink raw reply related [flat|nested] 8+ messages in thread