gh-108948: tarfile should handle sticky bit in FreeBSD

Lib/tarfile.py

@@ -37,6 +37,7 @@
 # Imports
 #---------
 from builtins import open as bltn_open
+import errno
 import sys
 import os
 import io
@@ -2567,7 +2568,26 @@ def chmod(self, tarinfo, targetpath):
         if tarinfo.mode is None:
             return
         try:
-            os.chmod(targetpath, tarinfo.mode)
+            try:
+                os.chmod(targetpath, tarinfo.mode)
+            except OSError as exc1:
+                # gh-108948: On FreeBSD, chmod() fails when trying to set the
+                # sticky bit on a file as non-root. On other platforms, the bit
+                # is silently ignored if it cannot be set. We make FreeBSD
+                # behave like other platforms by catching the error and trying
+                # again without the sticky bit.
+                if (hasattr(errno, "EFTYPE") and exc1.errno == errno.EFTYPE
+                    and (tarinfo.mode & stat.S_ISVTX)):
+                    try:
+                        # Retry without the sticky bit
+                        os.chmod(targetpath, tarinfo.mode & ~stat.S_ISVTX)
+                    except OSError as exc2:
+                        # The error from the second attempt is the direct cause
+                        # of the ExtractError, but we keep the original error
+                        # around for good information.
+                        raise exc2 from exc1
+                else:
+                    raise
         except OSError as e:
             raise ExtractError("could not change mode") from e
 

Lib/test/test_tarfile.py

@@ -9,6 +9,7 @@
 import re
 import warnings
 import stat
+import errno
 
 import unittest
 import unittest.mock
@@ -690,6 +691,36 @@ def format_mtime(mtime):
             tar.close()
             os_helper.rmtree(DIR)
 
+    @unittest.skipUnless(hasattr(errno, "EFTYPE"), "errno.EFTYPE required")
+    def test_extract_chmod_eftype(self):
+        # Extracting a file as non-root should skip the sticky bit (gh-108948)
+        # even on platforms where chmod fails with EFTYPE (i.e. FreeBSD). But
+        # we need to take care that any other error is preserved.
+        mode = "-rwxrwxrwt"
+        with ArchiveMaker() as arc:
+            arc.add("sticky1", mode=mode)
+            arc.add("sticky2", mode=mode)
+        tar = arc.open(errorlevel=2)
+        DIR = os.path.join(TEMPDIR, "chmod")
+        os.mkdir(DIR)
+        self.addCleanup(os_helper.rmtree, DIR)
+        with tar:
+            # this should not raise:
+            tar.extract("sticky1", DIR, filter="fully_trusted")
+            got_mode = stat.filemode(os.stat(os.path.join(DIR, "sticky1")).st_mode)
+            expected_mode = "-rwxrwxrwx" if os.geteuid() != 0 else "-rwxrwxrwt"
+            self.assertEqual(got_mode, expected_mode)
+
+            # but we can create a situation where it does raise:
+            with unittest.mock.patch("os.chmod") as mock:
+                eftype_error = OSError(errno.EFTYPE, "EFTYPE")
+                other_error = OSError(errno.EPERM, "different error")
+                mock.side_effect = [eftype_error, other_error]
+                with self.assertRaises(tarfile.ExtractError) as excinfo:
+                    tar.extract("sticky2", DIR, filter="fully_trusted")
+            self.assertEqual(excinfo.exception.__cause__, other_error)
+            self.assertEqual(excinfo.exception.__cause__.__cause__, eftype_error)
+
     @os_helper.skip_unless_working_chmod
     def test_extract_directory(self):
         dirtype = "ustar/dirtype"
@@ -3818,7 +3849,17 @@ def test_modes(self):
             pass
         new_mode = (os.stat(tmp_filename).st_mode
                     | stat.S_ISVTX | stat.S_ISGID | stat.S_ISUID)
-        os.chmod(tmp_filename, new_mode)
+        try:
+            os.chmod(tmp_filename, new_mode)
+        except OSError as err:
+            # gh-108948: While chmod on most platforms silently ignores the
+            # sticky bit if it cannot be set (i.e. setting it on a file as
+            # non-root), chmod on FreeBSD raises EFTYPE to indicate the case.
+            if hasattr(errno, "EFTYPE") and err.errno == errno.EFTYPE:
+                # Retry without the sticky bit
+                os.chmod(tmp_filename, new_mode & ~stat.S_ISVTX)
+            else:
+                raise
         got_mode = os.stat(tmp_filename).st_mode
         _t_file = 't' if (got_mode & stat.S_ISVTX) else 'x'
         _suid_file = 's' if (got_mode & stat.S_ISUID) else 'x'

Misc/NEWS.d/next/Library/2023-09-05-17-11-17.gh-issue-108948.HL1Ttw.rst

@@ -0,0 +1 @@
+On FreeBSD, :mod:`tarfile` will not attempt to set the sticky bit on extracted files when it's not possible, matching the behavior of other platforms.