fix(tar): reject symlink targets that resolve outside destination

lib/mixlib/archive/lib_archive.rb

@@ -18,7 +18,12 @@ def initialize(archive, options = {})
       # ignore[Array]:: an array of matches of file paths to ignore
       def extract(destination, perms: true, ignore: [])
         ignore_re = Regexp.union(ignore)
-        flags = perms ? ::Archive::EXTRACT_PERM : nil
+        # EXTRACT_SECURE_SYMLINKS refuses to write through a symlink (including
+        # one from an earlier entry in this same archive) that would otherwise
+        # let a later entry escape destination (tar-slip). EXTRACT_SECURE_NODOTDOT
+        # closes the equivalent ".." based escape.
+        flags = ::Archive::EXTRACT_SECURE_SYMLINKS | ::Archive::EXTRACT_SECURE_NODOTDOT
+        flags |= ::Archive::EXTRACT_PERM if perms
         FileUtils.mkdir_p(destination)
 
         reader = ::Archive::Reader.open_filename(@archive)

lib/mixlib/archive/tar.rb

@@ -47,7 +47,7 @@ def extract(destination, perms: true, ignore: [])
               next
             end
             parent = File.dirname(dest)
-            FileUtils.mkdir_p(parent)
+            FileUtils.mkdir_p(parent) unless File.directory?(parent)
 
             if entry.directory? || (entry.header.typeflag == "" && entry.full_name.end_with?("/"))
               File.delete(dest) if File.file?(dest)
@@ -66,7 +66,19 @@ def extract(destination, perms: true, ignore: [])
               FileUtils.chmod(entry.header.mode, dest, verbose: false) if perms
             elsif entry.header.typeflag == "2"
               # handle symlink
-              File.symlink(entry.header.linkname, dest)
+              #
+              # A symlink whose target resolves outside destination would let a
+              # *later* entry that writes "through" this symlink escape the
+              # destination directory entirely (tar-slip). Since every path we
+              # write is already confined to destination_root, refusing to create
+              # symlinks whose own target escapes destination_root guarantees no
+              # chain of symlinks can ever lead outside of it either.
+              link_target = File.expand_path(entry.header.linkname, File.dirname(dest))
+              if link_target == destination_root || link_target.start_with?(destination_root + File::SEPARATOR)
+                File.symlink(entry.header.linkname, dest)
+              else
+                Mixlib::Archive::Log.warn "ignoring entry #{entry.full_name}: symlink target #{entry.header.linkname} resolves outside #{destination}"
+              end
             else
               Mixlib::Archive::Log.warn "unknown tar entry: #{entry.full_name} type: #{entry.header.typeflag}"
             end

spec/mixlib/lib_archive_spec.rb

@@ -49,7 +49,7 @@
           Mixlib::Archive::LibArchive.new(archive_path).create(file_paths, gzip: true)
         end
         expect(File.file?(archive_path)).to be true
-        Mixlib::Archive::LibArchive.new(archive_path).extract(target)
+        Mixlib::Archive::LibArchive.new(archive_path).extract(target, ignore: %w{ . .. })
         expect(Dir.entries(target)).to match_array file_paths
         expect(File.size("#{target}/fixture_binary")).to eql 6
         expect(File.size("#{target}/fixture_a")).to eql 10

spec/mixlib/tar_spec.rb

@@ -118,6 +118,49 @@ def write_longlink_tar(archive_path, longlink_target, payload)
 
       expect(File.read(File.join(target, long_name))).to eq("hello")
     end
+
+    # Builds a tar with a symlink entry pointing outside of the extraction
+    # destination, followed by a regular file entry whose path travels
+    # "through" that symlink. Without a containment check on symlink targets
+    # this is the classic tar-slip attack: the symlink is planted first, then
+    # the second entry's write follows it to land anywhere on disk.
+    def write_symlink_escape_tar(archive_path, link_name, link_target, file_through_link, payload)
+      File.open(archive_path, "wb") do |f|
+        writer = Gem::Package::TarWriter.new(f)
+        writer.add_symlink(link_name, link_target, 0o777)
+        writer.add_file_simple(file_through_link, 0o644, payload.bytesize) { |io| io.write(payload) }
+        writer.close
+      end
+    end
+
+    it "does not create a symlink that escapes destination" do
+      write_symlink_escape_tar(archive_path, "link", outside_path, "link/pwned.txt", "pwned!")
+
+      described_class.new(archive_path).extract(target)
+
+      expect(File.symlink?(File.join(target, "link"))).to be false
+      expect(File.exist?(outside_path)).to be false
+    end
+
+    it "does not write a file outside destination via a symlinked path" do
+      FileUtils.mkdir_p(outside_path)
+      write_symlink_escape_tar(archive_path, "link", outside_path, "link/pwned.txt", "pwned!")
+
+      described_class.new(archive_path).extract(target)
+
+      expect(File.exist?(File.join(outside_path, "pwned.txt"))).to be false
+    end
+
+    it "still allows symlinks that resolve within destination" do
+      real_dir = File.join(target, "realdir")
+      FileUtils.mkdir_p(real_dir)
+      write_symlink_escape_tar(archive_path, "link", real_dir, "link/ok.txt", "hello")
+
+      described_class.new(archive_path).extract(target)
+
+      expect(File.symlink?(File.join(target, "link"))).to be true
+      expect(File.read(File.join(target, "realdir", "ok.txt"))).to eq("hello")
+    end
   end
 
   describe "#is_tar_archive?" do