Resolve relative path vulnerability

Fixes #16, CVE-2016-10173 Also makes the move from minitar.gemspec to archive-tar-minitar.gemspec.
halostatue · Feb 6, 2017 · 407a4ad · 407a4ad
1 parent 27569df
commit 407a4ad
Show file tree

Hide file tree

Showing 9 changed files with 185 additions and 57 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -1,12 +1,13 @@
 ---
 language: ruby
 rvm:
-  - 2.3.1
-  - 2.2.3
-  - 2.1.6
+  - 2.4.0
+  - 2.3.3
+  - 2.2.6
+  - 2.1.9
   - 2.0.0
   - jruby-9.0.5.0
-  - jruby-9.1.5.0
+  - jruby-9.1.6.0
   - 1.9.3
   - 1.8.7
   - ree
@@ -20,12 +21,6 @@ matrix:
 gemfile:
   - Gemfile
 before_script:
-  - |
-    case "${TRAVIS_RUBY_VERSION}" in
-      rbx*)
-        gem install psych
-        ;;
-    esac
   - rake travis:before -t
 script: rake travis
 after_script:

diff --git 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/Rakefile b/Rakefile
@@ -4,17 +4,20 @@ require 'rubygems'
 require 'hoe'
 require 'rake/clean'
 
+$LOAD_PATH.unshift('support')
+
 Hoe.plugin :doofus
 Hoe.plugin :gemspec2
 Hoe.plugin :git
 Hoe.plugin :minitest
 Hoe.plugin :travis
+Hoe.plugin :deprecated_gem
 Hoe.plugin :email unless ENV['CI'] or ENV['TRAVIS']
 
 spec = Hoe.spec 'minitar' do
   developer('Austin Ziegler', 'halostatue@gmail.com')
 
-  self.require_ruby_version '>= 1.8'
+  require_ruby_version '>= 1.8'
 
   self.history_file = 'History.md'
   self.readme_file = 'README.rdoc'
@@ -26,8 +29,8 @@ spec = Hoe.spec 'minitar' do
   extra_dev_deps << ['hoe-rubygems', '~> 1.0']
   extra_dev_deps << ['hoe-travis', '~> 1.2']
   extra_dev_deps << ['minitest', '~> 5.3']
-  extra_dev_deps << ['minitest-autotest', ['>= 1.0.b', '<2']]
-  extra_dev_deps << ['rake', '~> 10.0']
+  extra_dev_deps << ['minitest-autotest', ['>= 1.0', '<2']]
+  extra_dev_deps << ['rake', '>= 10.0', '< 12']
   extra_dev_deps << ['rdoc', '>= 0.0']
 end
 

diff --git a/archive-tar-minitar.gemspec b/archive-tar-minitar.gemspec
@@ -1,15 +1,36 @@
 # -*- encoding: utf-8 -*-
 # stub: archive-tar-minitar 0.6 ruby lib
 
-minitar = Gem::Specification.load('minitar.gemspec')
-minitar.name = 'archive-tar-minitar'
-minitar.description =
-  minitar.summary = %q(This gem is deprecated. Just install 'minitar'.)
-minitar.files.delete_if { |f| f !~ %r{lib/archive-tar-minitar\.rb} }
-minitar.extra_rdoc_files.clear
-minitar.rdoc_options.clear
-minitar.dependencies.clear
-minitar.add_dependency(%q<minitar>, "~> #{minitar.version}")
-minitar.add_dependency(%q<minitar-cli>, "<= 1.0")
-
-minitar
+Gem::Specification.new do |s|
+  s.name = "archive-tar-minitar"
+  s.version = "0.6"
+
+  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 = "2017-02-06"
+  s.description = "'archive-tar-minitar' has been deprecated; just install 'minitar'. 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.files = ["lib/archive-tar-minitar.rb"]
+  s.homepage = "https://github.com/halostatue/minitar/"
+  s.licenses = ["Ruby", "BSD-2-Clause"]
+  s.post_install_message = "'archive-tar-minitar' has been deprecated; just install 'minitar'."
+  s.required_ruby_version = Gem::Requirement.new(">= 1.8")
+  s.rubygems_version = "2.5.1"
+  s.summary = "'archive-tar-minitar' has been deprecated; just install 'minitar'."
+
+  if s.respond_to? :specification_version then
+    s.specification_version = 4
+
+    if Gem::Version.new(Gem::VERSION) >= Gem::Version.new('1.2.0') then
+      s.add_runtime_dependency(%q<minitar>, ["~> 0.6"])
+      s.add_runtime_dependency(%q<minitar-cli>, ["<= 1.0"])
+    else
+      s.add_dependency(%q<minitar>, ["~> 0.6"])
+      s.add_dependency(%q<minitar-cli>, ["<= 1.0"])
+    end
+  else
+    s.add_dependency(%q<minitar>, ["~> 0.6"])
+    s.add_dependency(%q<minitar-cli>, ["<= 1.0"])
+  end
+end
diff --git 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
@@ -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
@@ -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 = "2017-02-06"
   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"]
@@ -30,21 +30,21 @@ Gem::Specification.new do |s|
       s.add_development_dependency(%q<hoe-git>, ["~> 1.6"])
       s.add_development_dependency(%q<hoe-rubygems>, ["~> 1.0"])
       s.add_development_dependency(%q<hoe-travis>, ["~> 1.2"])
-      s.add_development_dependency(%q<minitest-autotest>, ["< 2", ">= 1.0.b"])
-      s.add_development_dependency(%q<rake>, ["~> 10.0"])
+      s.add_development_dependency(%q<minitest-autotest>, ["< 2", ">= 1.0"])
+      s.add_development_dependency(%q<rake>, ["< 12", ">= 10.0"])
       s.add_development_dependency(%q<rdoc>, [">= 0.0"])
-      s.add_development_dependency(%q<hoe>, ["~> 3.15"])
+      s.add_development_dependency(%q<hoe>, ["~> 3.16"])
     else
       s.add_dependency(%q<minitest>, ["~> 5.9"])
       s.add_dependency(%q<hoe-doofus>, ["~> 1.0"])
       s.add_dependency(%q<hoe-gemspec2>, ["~> 1.1"])
       s.add_dependency(%q<hoe-git>, ["~> 1.6"])
       s.add_dependency(%q<hoe-rubygems>, ["~> 1.0"])
       s.add_dependency(%q<hoe-travis>, ["~> 1.2"])
-      s.add_dependency(%q<minitest-autotest>, ["< 2", ">= 1.0.b"])
-      s.add_dependency(%q<rake>, ["~> 10.0"])
+      s.add_dependency(%q<minitest-autotest>, ["< 2", ">= 1.0"])
+      s.add_dependency(%q<rake>, ["< 12", ">= 10.0"])
       s.add_dependency(%q<rdoc>, [">= 0.0"])
-      s.add_dependency(%q<hoe>, ["~> 3.15"])
+      s.add_dependency(%q<hoe>, ["~> 3.16"])
     end
   else
     s.add_dependency(%q<minitest>, ["~> 5.9"])
@@ -53,9 +53,9 @@ Gem::Specification.new do |s|
     s.add_dependency(%q<hoe-git>, ["~> 1.6"])
     s.add_dependency(%q<hoe-rubygems>, ["~> 1.0"])
     s.add_dependency(%q<hoe-travis>, ["~> 1.2"])
-    s.add_dependency(%q<minitest-autotest>, ["< 2", ">= 1.0.b"])
-    s.add_dependency(%q<rake>, ["~> 10.0"])
+    s.add_dependency(%q<minitest-autotest>, ["< 2", ">= 1.0"])
+    s.add_dependency(%q<rake>, ["< 12", ">= 10.0"])
     s.add_dependency(%q<rdoc>, [">= 0.0"])
-    s.add_dependency(%q<hoe>, ["~> 3.15"])
+    s.add_dependency(%q<hoe>, ["~> 3.16"])
   end
 end
diff --git a/support/hoe/deprecated_gem.rb b/support/hoe/deprecated_gem.rb
@@ -0,0 +1,39 @@
+module Hoe::Deprecated_Gem
+  def linked_spec(spec)
+    atm = YAML.load(YAML.dump(spec))
+    atm.name = 'archive-tar-minitar'
+    d = %Q('#{atm.name}' has been deprecated; just install '#{spec.name}'.)
+    atm.description = "#{d} #{spec.description}"
+    atm.summary = atm.post_install_message = d
+    atm.files.delete_if { |f| f !~ %r{lib/archive-tar-minitar\.rb} }
+    atm.extra_rdoc_files.clear
+    atm.rdoc_options.clear
+    atm.dependencies.clear
+    atm.add_dependency(%Q(#{spec.name}), "~> #{spec.version}")
+    atm.add_dependency(%Q(#{spec.name}-cli), "<= 1.0")
+
+    unless @include_all
+      [ :signing_key, :cert_chain ].each { |name|
+        atm.send("#{name}=".to_sym, atm.default_value(name))
+      }
+    end
+
+    atm
+  end
+
+  def define_deprecated_gem_tasks
+    gemspec = spec.name + '.gemspec'
+    atmspec = 'archive-tar-minitar.gemspec'
+
+    file atmspec => gemspec do
+      open(atmspec, 'w') { |f| f.write(linked_spec(spec).to_ruby) }
+    end
+
+    task :gemspec => atmspec
+
+    Gem::PackageTask.new linked_spec(spec) do |pkg|
+      pkg.need_tar = @need_tar
+      pkg.need_zip = @need_zip
+    end
+  end
+end