gh-59999: zipfile: restore Unix permissions after extraction

Doc/library/zipfile.rst

@@ -151,6 +151,44 @@ The module defines the following items:
 
    .. versionadded:: 3.14
 
+.. class:: PreserveMode
+
+   An :class:`~enum.Enum` of the values accepted by the *preserve_permissions*
+   parameter of :meth:`ZipFile.extract` and :meth:`ZipFile.extractall`.  It
+   controls how the Unix permissions stored in the upper 16 bits of
+   :attr:`ZipInfo.external_attr` are applied to the extracted files and
+   directories.  The stored permissions are used as they are; the current
+   umask is not applied.
+
+   These members always get the default permissions: members with no stored
+   Unix permissions, symbolic links, and members created on MS-DOS or Windows
+   (:attr:`ZipInfo.create_system` is ``0``).
+
+   .. attribute:: NONE
+
+      Do not preserve permissions.  Extracted files and directories get the
+      default permissions for newly created files and directories.  This is
+      the default.
+
+   .. attribute:: SAFE
+
+      Preserve only the read, write and execute permissions for the user,
+      group and others (``0o777``).  The set-user-ID, set-group-ID and
+      sticky bits are dropped.
+
+   .. attribute:: ALL
+
+      Preserve all permission bits (``0o7777``), including the set-user-ID
+      (:data:`stat.S_ISUID`), set-group-ID (:data:`stat.S_ISGID`) and sticky
+      (:data:`stat.S_ISVTX`) bits.
+
+      .. warning::
+
+         Only use this for archives from trusted sources.  An archive can
+         make extracted programs run with the privileges of their owner.
+
+   .. versionadded:: next
+
 .. note::
 
    The ZIP file format specification has included support for bzip2 compression
@@ -377,14 +415,21 @@ ZipFile objects
       object was changed from ``'r'`` to ``'rb'``.
 
 
-.. method:: ZipFile.extract(member, path=None, pwd=None)
+.. method:: ZipFile.extract(member, path=None, pwd=None, *, \
+                            preserve_permissions=PreserveMode.NONE)
 
    Extract a member from the archive to the current working directory; *member*
    must be its full name or a :class:`ZipInfo` object.  Its file information is
    extracted as accurately as possible.  *path* specifies a different directory
    to extract to.  *member* can be a filename or a :class:`ZipInfo` object.
    *pwd* is the password used for encrypted files as a :class:`bytes` object.
 
+   *preserve_permissions* is a :class:`PreserveMode` value that controls
+   whether the Unix permissions stored in the archive are applied to the
+   extracted file or directory.  By default, they are not.  If a read-only
+   directory is extracted first, extracting its contents afterwards will
+   fail; :meth:`extractall` avoids this.
+
    Returns the normalized path created (a directory or new file).
 
    .. note::
@@ -404,14 +449,23 @@ ZipFile objects
    .. versionchanged:: 3.6.2
       The *path* parameter accepts a :term:`path-like object`.
 
+   .. versionchanged:: next
+      Added the *preserve_permissions* parameter.
+
 
-.. method:: ZipFile.extractall(path=None, members=None, pwd=None)
+.. method:: ZipFile.extractall(path=None, members=None, pwd=None, *, \
+                               preserve_permissions=PreserveMode.NONE)
 
    Extract all members from the archive to the current working directory.  *path*
    specifies a different directory to extract to.  *members* is optional and must
    be a subset of the list returned by :meth:`namelist`.  *pwd* is the password
    used for encrypted files as a :class:`bytes` object.
 
+   *preserve_permissions* has the same meaning as for :meth:`extract`.
+   Permissions of directories are applied after all members have been
+   extracted, so that a read-only directory does not prevent its contents
+   from being extracted.
+
    .. warning::
 
       Never extract archives from untrusted sources without prior inspection.
@@ -427,6 +481,9 @@ ZipFile objects
    .. versionchanged:: 3.6.2
       The *path* parameter accepts a :term:`path-like object`.
 
+   .. versionchanged:: next
+      Added the *preserve_permissions* parameter.
+
 
 .. method:: ZipFile.printdir()
 

Lib/test/test_zipfile/test_core.py

@@ -3968,6 +3968,106 @@ def _test_extract_hackers_arcnames(self, hacknames):
             unlink(TESTFN2)
 
 
+@unittest.skipIf(sys.platform == 'win32', 'requires Unix permissions')
+@os_helper.skip_unless_working_chmod
+class PreservePermissionsTests(unittest.TestCase):
+    # File modes saved in the test archive.
+    FILE_MODES = {
+        'plain': 0o644,
+        'private': 0o600,
+        'script': 0o755,
+        'setuid': 0o4755,
+        'setgid': 0o2755,
+        'sticky': 0o1644,
+    }
+
+    def setUp(self):
+        # Fixed umask, so we know the default mode (0o644).
+        self.enterContext(os_helper.temp_umask(0o022))
+        self.addCleanup(unlink, TESTFN2)
+
+    def make_zip(self, members, dirs=()):
+        # members maps a name to (external_attr, create_system).
+        with zipfile.ZipFile(TESTFN2, 'w') as zf:
+            for name, mode in dirs:
+                zf.mkdir(name, mode)
+            for name, (external_attr, create_system) in members.items():
+                zinfo = zipfile.ZipInfo(name)
+                zinfo.external_attr = external_attr
+                zinfo.create_system = create_system
+                zf.writestr(zinfo, name)
+
+    def make_unix_zip(self, file_modes, dirs=()):
+        # The Unix mode goes in the top 16 bits; 3 means made on Unix.
+        self.make_zip({name: ((stat.S_IFREG | mode) << 16, 3)
+                       for name, mode in file_modes.items()}, dirs)
+
+    def check_extract(self, expected, methods=('extract', 'extractall'),
+                      **kwargs):
+        for method in methods:
+            with self.subTest(method=method, **kwargs), temp_dir() as extdir:
+                with zipfile.ZipFile(TESTFN2) as zf:
+                    if method == 'extract':
+                        for name in zf.namelist():
+                            zf.extract(name, extdir, **kwargs)
+                    else:
+                        zf.extractall(extdir, **kwargs)
+                for name, mode in expected.items():
+                    st = os.stat(os.path.join(extdir, name))
+                    self.assertEqual(oct(stat.S_IMODE(st.st_mode)), oct(mode),
+                                     name)
+
+    def test_default(self):
+        self.make_unix_zip(self.FILE_MODES)
+        expected = dict.fromkeys(self.FILE_MODES, 0o644)
+        self.check_extract(expected)
+        self.check_extract(expected,
+                           preserve_permissions=zipfile.PreserveMode.NONE)
+
+    def test_preserve_safe(self):
+        self.make_unix_zip(self.FILE_MODES)
+        expected = {name: mode & 0o777
+                    for name, mode in self.FILE_MODES.items()}
+        self.check_extract(expected,
+                           preserve_permissions=zipfile.PreserveMode.SAFE)
+
+    def test_preserve_all(self):
+        # No setgid or sticky here: normal users can't always set them.
+        file_modes = {'private': 0o600, 'script': 0o755, 'setuid': 0o4755}
+        self.make_unix_zip(file_modes)
+        self.check_extract(file_modes,
+                           preserve_permissions=zipfile.PreserveMode.ALL)
+
+    def test_mode_not_used(self):
+        # These members always get the default mode.
+        self.make_zip({
+            'no_mode': (0x20, 3),  # only a DOS flag, no Unix mode
+            'windows': ((stat.S_IFREG | 0o755) << 16, 0),  # made on Windows
+            'symlink': ((stat.S_IFLNK | 0o777) << 16, 3),  # a symlink
+        })
+        expected = dict.fromkeys(['no_mode', 'windows', 'symlink'], 0o644)
+        self.check_extract(expected,
+                           preserve_permissions=zipfile.PreserveMode.ALL)
+
+    def test_read_only_directory(self):
+        # extractall() sets directory modes last, so the file can still
+        # be written into the read-only directory.
+        self.make_unix_zip({'ro/file': 0o644}, dirs=[('ro', 0o555)])
+        self.check_extract({'ro': 0o555, 'ro/file': 0o644},
+                           methods=['extractall'],
+                           preserve_permissions=zipfile.PreserveMode.SAFE)
+
+    def test_bad_value(self):
+        self.make_unix_zip({'plain': 0o644})
+        with zipfile.ZipFile(TESTFN2) as zf, temp_dir() as extdir:
+            with self.assertRaises(TypeError):
+                zf.extract('plain', extdir, preserve_permissions='SAFE')
+            with self.assertRaises(TypeError):
+                zf.extractall(extdir, preserve_permissions='SAFE')
+            # Nothing was extracted.
+            self.assertEqual(os.listdir(extdir), [])
+
+
 class OverwriteTests(archiver_tests.OverwriteTests, unittest.TestCase):
     testdir = TESTFN
 

Lib/zipfile/__init__.py

@@ -4,6 +4,7 @@
 XXX references to utf-8 need further investigation.
 """
 import binascii
+import enum
 import io
 import os
 import shutil
@@ -38,8 +39,8 @@
 
 __all__ = ["BadZipFile", "BadZipfile", "error",
            "ZIP_STORED", "ZIP_DEFLATED", "ZIP_BZIP2", "ZIP_LZMA",
-           "ZIP_ZSTANDARD", "is_zipfile", "ZipInfo", "ZipFile", "PyZipFile",
-           "LargeZipFile", "Path"]
+           "ZIP_ZSTANDARD", "PreserveMode", "is_zipfile", "ZipInfo", "ZipFile",
+           "PyZipFile", "LargeZipFile", "Path"]
 
 class BadZipFile(Exception):
     pass
@@ -430,6 +431,26 @@ def _read_local_file_header(fp):
     return fheader
 
 
+class PreserveMode(enum.Enum):
+    """Which stored file permissions to apply when extracting."""
+    NONE = enum.auto()
+    SAFE = enum.auto()
+    ALL = enum.auto()
+
+
+_PRESERVE_MODE_MASKS = {
+    PreserveMode.NONE: 0,
+    PreserveMode.SAFE: 0o777,
+    PreserveMode.ALL: 0o7777,
+}
+
+
+def _check_preserve_mode(preserve_permissions):
+    if not isinstance(preserve_permissions, PreserveMode):
+        raise TypeError("preserve_permissions must be a zipfile.PreserveMode, "
+                        f"not {type(preserve_permissions).__name__}")
+
+
 class ZipInfo:
     """Class with attributes describing each file in the ZIP archive."""
 
@@ -2346,27 +2367,36 @@ def _open_to_write(self, zinfo, force_zip64=False):
         self._writing = True
         return _ZipWriteFile(self, zinfo, zip64)
 
-    def extract(self, member, path=None, pwd=None):
+    def extract(self, member, path=None, pwd=None, *,
+                preserve_permissions=PreserveMode.NONE):
         """Extract a member from the archive to the current working directory,
            using its full name. Its file information is extracted as accurately
            as possible. 'member' may be a filename or a ZipInfo object. You can
            specify a different directory using 'path'. You can specify the
-           password to decrypt the file using 'pwd'.
+           password to decrypt the file using 'pwd'. 'preserve_permissions'
+           chooses which stored Unix permissions to apply.
         """
+        _check_preserve_mode(preserve_permissions)
         if path is None:
             path = os.getcwd()
         else:
             path = os.fspath(path)
 
-        return self._extract_member(member, path, pwd)
+        targetpath = self._extract_member(member, path, pwd)
+        if targetpath is not None:
+            self._apply_permissions(member, targetpath, preserve_permissions)
+        return targetpath
 
-    def extractall(self, path=None, members=None, pwd=None):
+    def extractall(self, path=None, members=None, pwd=None, *,
+                   preserve_permissions=PreserveMode.NONE):
         """Extract all members from the archive to the current working
            directory. 'path' specifies a different directory to extract to.
            'members' is optional and must be a subset of the list returned
            by namelist(). You can specify the password to decrypt all files
-           using 'pwd'.
+           using 'pwd'. 'preserve_permissions' chooses which stored Unix
+           permissions to apply.
         """
+        _check_preserve_mode(preserve_permissions)
         if members is None:
             members = self.namelist()
 
@@ -2375,8 +2405,24 @@ def extractall(self, path=None, members=None, pwd=None):
         else:
             path = os.fspath(path)
 
+        directories = []
         for zipinfo in members:
-            self._extract_member(zipinfo, path, pwd)
+            targetpath = self._extract_member(zipinfo, path, pwd)
+            if targetpath is None or preserve_permissions is PreserveMode.NONE:
+                continue
+            if not isinstance(zipinfo, ZipInfo):
+                zipinfo = self.getinfo(zipinfo)
+            if zipinfo.is_dir():
+                directories.append((targetpath, zipinfo))
+            else:
+                self._apply_permissions(zipinfo, targetpath,
+                                        preserve_permissions)
+
+        # Set directory modes last, deepest first, so a read-only
+        # directory doesn't stop files being written into it.
+        directories.sort(key=lambda item: item[0], reverse=True)
+        for targetpath, zipinfo in directories:
+            self._apply_permissions(zipinfo, targetpath, preserve_permissions)
 
     def remove(self, zinfo_or_arcname):
         """Remove a member from the archive."""
@@ -2465,6 +2511,20 @@ def _sanitize_windows_name(cls, arcname, pathsep):
         arcname = pathsep.join(x for x in arcname if x)
         return arcname
 
+    def _apply_permissions(self, member, targetpath, preserve_permissions):
+        """Set the Unix permissions stored for 'member' on targetpath."""
+        mask = _PRESERVE_MODE_MASKS[preserve_permissions]
+        if not mask:
+            return
+        if not isinstance(member, ZipInfo):
+            member = self.getinfo(member)
+        # Made on MS-DOS/Windows: there is no real Unix mode.
+        if member.create_system == 0:
+            return
+        mode = member.external_attr >> 16
+        if mode and not stat.S_ISLNK(mode):
+            os.chmod(targetpath, mode & mask)
+
     def _extract_member(self, member, targetpath, pwd):
         """Extract the ZipInfo object 'member' to a physical
            file on the path targetpath.