From mboxrd@z Thu Jan 1 00:00:00 1970 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 Message-ID: <63fbd81e15aa5449286e39040d23555d4a2f21e7.1485287714.git.leo@famulari.name> References: Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:58416) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cW7DV-0007Ht-Hr for guix-devel@gnu.org; Tue, 24 Jan 2017 14:57:23 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cW7DR-0000u7-8E for guix-devel@gnu.org; Tue, 24 Jan 2017 14:57:21 -0500 Received: from out1-smtp.messagingengine.com ([66.111.4.25]:47413) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cW7DR-0000ta-2h for guix-devel@gnu.org; Tue, 24 Jan 2017 14:57:17 -0500 Received: from localhost.localdomain (c-73-188-17-148.hsd1.pa.comcast.net [73.188.17.148]) by mail.messagingengine.com (Postfix) with ESMTPA id 1EBC87E448 for ; Tue, 24 Jan 2017 14:57:15 -0500 (EST) In-Reply-To: In-Reply-To: References: List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: "Guix-devel" To: guix-devel@gnu.org * 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 +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