From 20e729f9a5d52aaa5a01aa4712ebfeb3ed6c2db0 Mon Sep 17 00:00:00 2001
From: Hiroshi SHIBATA <hsbt@ruby-lang.org>
Date: Mon, 3 Aug 2026 11:38:39 +0900
Subject: [PATCH] Check the resolved parent directory before extracting old
 format gems

Gem::Package::Old#extract_files only validated entry paths by string
expansion, so a preexisting symlink in the extraction directory redirected
writes outside of it. Gem::Package#extract_tar_gz already re-resolves the
parent directory with File.realpath, so extract the shared check into
verify_extraction_dir and call it from both.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 lib/rubygems/package.rb               | 19 +++++++++++++++----
 lib/rubygems/package/old.rb           |  6 ++++--
 test/rubygems/test_gem_package_old.rb | 16 ++++++++++++++++
 3 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/lib/rubygems/package.rb b/lib/rubygems/package.rb
index 4e9c470ec195..e1a80717dec6 100644
--- a/lib/rubygems/package.rb
+++ b/lib/rubygems/package.rb
@@ -453,10 +453,7 @@ def extract_tar_gz(io, destination_dir, pattern = "*") # :nodoc:
           directories << mkdir
         end
 
-        real_mkdir = File.realpath(mkdir)
-        unless real_mkdir == destination_dir || normalize_path(real_mkdir).start_with?(normalize_path(destination_dir + "/"))
-          raise Gem::Package::PathError.new(real_mkdir, destination_dir)
-        end
+        verify_extraction_dir mkdir, destination_dir
 
         if entry.file?
           File.open(destination, "wb") do |out|
@@ -662,6 +659,20 @@ def install_location(filename, destination_dir) # :nodoc:
     destination
   end
 
+  ##
+  # Raises an exception unless +dir+, with symlinks resolved, is
+  # +destination_dir+ or a directory inside it.  +destination_dir+ must
+  # already be resolved with File.realpath by the caller.
+
+  def verify_extraction_dir(dir, destination_dir) # :nodoc:
+    real_dir = File.realpath(dir)
+
+    return if real_dir == destination_dir ||
+              normalize_path(real_dir).start_with?(normalize_path(destination_dir + "/"))
+
+    raise Gem::Package::PathError.new(real_dir, destination_dir)
+  end
+
   ##
   # Verifies the +checksums+ against the +digests+.  This check is not
   # cryptographically secure.  Missing checksums are ignored.
diff --git a/lib/rubygems/package/old.rb b/lib/rubygems/package/old.rb
index e06be05bc0a1..27115fd2f9ea 100644
--- a/lib/rubygems/package/old.rb
+++ b/lib/rubygems/package/old.rb
@@ -79,10 +79,12 @@ def extract_files(destination_dir)
         raise Gem::Package::FormatError, "#{full_name} in #{@gem} is corrupt" if
           file_data.length != entry["size"].to_i
 
-        FileUtils.rm_rf destination
-
         FileUtils.mkdir_p File.dirname(destination), mode: dir_mode && 0o755
 
+        verify_extraction_dir File.dirname(destination), destination_dir
+
+        FileUtils.rm_rf destination
+
         File.open destination, "wb", file_mode(entry["mode"]) do |out|
           out.write file_data
         end
diff --git a/test/rubygems/test_gem_package_old.rb b/test/rubygems/test_gem_package_old.rb
index e532fa25e1b1..562f9f7c5103 100644
--- a/test/rubygems/test_gem_package_old.rb
+++ b/test/rubygems/test_gem_package_old.rb
@@ -44,6 +44,22 @@ def test_extract_files
       assert_equal mask, File.stat(extracted).mode unless Gem.win_platform?
     end
 
+    def test_extract_files_rejects_preexisting_symlink_escape
+      omit "Symlinks not supported or not enabled" unless symlink_supported?
+
+      escape_dir = File.join @tempdir, "escape"
+      FileUtils.mkdir_p escape_dir
+
+      File.symlink escape_dir, File.join(@destination, "lib")
+
+      assert_raise Gem::Package::PathError do
+        @package.extract_files @destination
+      end
+
+      assert_path_not_exist File.join(escape_dir, "foo.rb"),
+                            "must not write outside extraction root via symlink"
+    end
+
     def test_extract_files_security_policy
       pend "openssl is missing" unless Gem::HAVE_OPENSSL