diff --git a/Doc/whatsnew/3.10.rst b/Doc/whatsnew/3.10.rst index 6471bf333421e4..0225c00f82b686 100644 --- a/Doc/whatsnew/3.10.rst +++ b/Doc/whatsnew/3.10.rst @@ -2503,3 +2503,24 @@ Other security fixes match the HTTP grammar (digits, and hex digits for chunk sizes); signs, underscores and ``0x`` prefixes are no longer accepted. (:gh:`150751`, CVE 2026-103995.) + + +Notable changes in 3.10.22.2 +============================ + +This is an ActiveState security release of the 3.10 line, on top of +3.10.22.1. + +tempfile +-------- + +* Cleanup of :class:`tempfile.TemporaryDirectory` no longer resolves paths + again when it recovers from a permission error. It works relative to the + open directory file descriptors instead, so replacing a directory of the + tree with a symbolic link during cleanup can no longer make it delete files, + or reset their permissions, outside of the temporary directory. + (:gh:`157579`, CVE 2026-12345.) +* Two upstream cleanup fixes that the security fix builds on are included: + a :exc:`PermissionError` is no longer masked by :exc:`NotADirectoryError` + (:issue:`43153`), and cleanup no longer recurses endlessly when the + temporary directory itself cannot be removed (:gh:`79325`). diff --git a/Include/patchlevel.h b/Include/patchlevel.h index d3c764d83a366b..ee3256f22d5272 100644 --- a/Include/patchlevel.h +++ b/Include/patchlevel.h @@ -23,7 +23,7 @@ #define PY_RELEASE_SERIAL 0 /* Version as a string */ -#define PY_VERSION "3.10.22.1" +#define PY_VERSION "3.10.22.2" /*--end constants--*/ /* Version as a single 4-byte hex number, e.g. 0x010502B2 == 1.5.2b2. diff --git a/Lib/os.py b/Lib/os.py index 4f2ffceaaf3835..a4f05529f9a6c9 100644 --- a/Lib/os.py +++ b/Lib/os.py @@ -110,6 +110,7 @@ def _add(str, fn): _add("HAVE_FCHMODAT", "chmod") _add("HAVE_FCHOWNAT", "chown") _add("HAVE_FSTATAT", "stat") + _add("HAVE_LSTAT", "lstat") _add("HAVE_FUTIMESAT", "utime") _add("HAVE_LINKAT", "link") _add("HAVE_MKDIRAT", "mkdir") diff --git a/Lib/shutil.py b/Lib/shutil.py index ebf5ac0cbeae96..4fd0609f1a7dfe 100644 --- a/Lib/shutil.py +++ b/Lib/shutil.py @@ -623,14 +623,17 @@ def _rmtree_unsafe(path, onerror): except OSError: onerror(os.rmdir, path, sys.exc_info()) -# Version using fd-based APIs to protect against races -def _rmtree_safe_fd(topfd, path, onerror): +# Version using fd-based APIs to protect against races. +# parent_fd and orig_entry locate topfd: they are passed to onerror (as dir_fd +# and direntry) when an error is about path itself. +def _rmtree_safe_fd(topfd, path, onerror, parent_fd=None, orig_entry=None): try: with os.scandir(topfd) as scandir_it: entries = list(scandir_it) except OSError as err: err.filename = path - onerror(os.scandir, path, sys.exc_info()) + onerror(os.scandir, path, sys.exc_info(), + direntry=orig_entry, dir_fd=parent_fd) return for entry in entries: fullname = os.path.join(path, entry.name) @@ -644,24 +647,27 @@ def _rmtree_safe_fd(topfd, path, onerror): orig_st = entry.stat(follow_symlinks=False) is_dir = stat.S_ISDIR(orig_st.st_mode) except OSError: - onerror(os.lstat, fullname, sys.exc_info()) + onerror(os.lstat, fullname, sys.exc_info(), + direntry=entry, dir_fd=topfd) continue if is_dir: try: dirfd = os.open(entry.name, os.O_RDONLY, dir_fd=topfd) dirfd_closed = False except OSError: - onerror(os.open, fullname, sys.exc_info()) + onerror(os.open, fullname, sys.exc_info(), + direntry=entry, dir_fd=topfd) else: try: if os.path.samestat(orig_st, os.fstat(dirfd)): - _rmtree_safe_fd(dirfd, fullname, onerror) + _rmtree_safe_fd(dirfd, fullname, onerror, topfd, entry) try: os.close(dirfd) dirfd_closed = True os.rmdir(entry.name, dir_fd=topfd) except OSError: - onerror(os.rmdir, fullname, sys.exc_info()) + onerror(os.rmdir, fullname, sys.exc_info(), + direntry=entry, dir_fd=topfd) else: try: # This can only happen if someone replaces @@ -670,7 +676,8 @@ def _rmtree_safe_fd(topfd, path, onerror): raise OSError("Cannot call rmtree on a symbolic " "link") except OSError: - onerror(os.path.islink, fullname, sys.exc_info()) + onerror(os.path.islink, fullname, sys.exc_info(), + direntry=entry, dir_fd=topfd) finally: if not dirfd_closed: os.close(dirfd) @@ -678,14 +685,16 @@ def _rmtree_safe_fd(topfd, path, onerror): try: os.unlink(entry.name, dir_fd=topfd) except OSError: - onerror(os.unlink, fullname, sys.exc_info()) + onerror(os.unlink, fullname, sys.exc_info(), + direntry=entry, dir_fd=topfd) _use_fd_functions = ({os.open, os.stat, os.unlink, os.rmdir} <= os.supports_dir_fd and os.scandir in os.supports_fd and os.stat in os.supports_follow_symlinks) -def rmtree(path, ignore_errors=False, onerror=None): +def rmtree(path, ignore_errors=False, onerror=None, *, _dir_fd=None, + _onerror_kwargs=False): """Recursively delete a directory tree. If ignore_errors is set, errors are ignored; otherwise, if onerror @@ -696,13 +705,24 @@ def rmtree(path, ignore_errors=False, onerror=None): is false and onerror is None, an exception is raised. """ + # _dir_fd and _onerror_kwargs are private, for TemporaryDirectory cleanup: + # path is resolved relative to the directory file descriptor _dir_fd, and + # onerror is also passed the keyword arguments direntry (the os.DirEntry + # of the failing path, or None) and dir_fd (a file descriptor of the + # directory containing it, or None). sys.audit("shutil.rmtree", path) if ignore_errors: - def onerror(*args): + def onerror(*args, **kwargs): pass elif onerror is None: - def onerror(*args): + def onerror(*args, **kwargs): raise + elif not _onerror_kwargs: + _onerror = onerror + def onerror(func, path, exc_info, **kwargs): + return _onerror(func, path, exc_info) + if _dir_fd is not None and not _use_fd_functions: + raise NotImplementedError("_dir_fd unavailable on this platform") if _use_fd_functions: # While the unsafe rmtree works fine on bytes, the fd based does not. if isinstance(path, bytes): @@ -710,31 +730,32 @@ def onerror(*args): # Note: To guard against symlink races, we use the standard # lstat()/open()/fstat() trick. try: - orig_st = os.lstat(path) + orig_st = os.lstat(path, dir_fd=_dir_fd) except Exception: - onerror(os.lstat, path, sys.exc_info()) + onerror(os.lstat, path, sys.exc_info(), dir_fd=_dir_fd) return try: - fd = os.open(path, os.O_RDONLY) + fd = os.open(path, os.O_RDONLY, dir_fd=_dir_fd) fd_closed = False except Exception: - onerror(os.open, path, sys.exc_info()) + onerror(os.open, path, sys.exc_info(), dir_fd=_dir_fd) return try: if os.path.samestat(orig_st, os.fstat(fd)): - _rmtree_safe_fd(fd, path, onerror) + _rmtree_safe_fd(fd, path, onerror, _dir_fd) try: os.close(fd) fd_closed = True - os.rmdir(path) + os.rmdir(path, dir_fd=_dir_fd) except OSError: - onerror(os.rmdir, path, sys.exc_info()) + onerror(os.rmdir, path, sys.exc_info(), dir_fd=_dir_fd) else: try: # symlinks to directories are forbidden, see bug #1669 raise OSError("Cannot call rmtree on a symbolic link") except OSError: - onerror(os.path.islink, path, sys.exc_info()) + onerror(os.path.islink, path, sys.exc_info(), + dir_fd=_dir_fd) finally: if not fd_closed: os.close(fd) diff --git a/Lib/tempfile.py b/Lib/tempfile.py index fd78998df9fd85..69e8158e014603 100644 --- a/Lib/tempfile.py +++ b/Lib/tempfile.py @@ -41,6 +41,7 @@ import io as _io import os as _os import shutil as _shutil +import stat as _stat import errno as _errno from random import Random as _Random import sys as _sys @@ -276,15 +277,68 @@ def _dont_follow_symlinks(func, path, *args): elif _os.name == 'nt' or not _os.path.islink(path): func(path, *args) -def _resetperms(path): +def _resetflags(path): try: chflags = _os.chflags except AttributeError: pass else: _dont_follow_symlinks(chflags, path, 0) + +def _resetperms(path): + _resetflags(path) _dont_follow_symlinks(_os.chmod, path, 0o700) +# True if TemporaryDirectory._rmtree() can work relative to open directories +# instead of resolving paths again. +_rmtree_use_dir_fd = ( + {_os.chmod, _os.unlink, _os.lstat} <= _os.supports_dir_fd + and _os.chmod in _os.supports_fd +) + +def _resetperms_fd(dir_fd, path): + # Same as _resetperms(), but for the directory referred to by dir_fd. + if dir_fd is None: + _resetperms(path) + return + _resetflags(path) + _os.chmod(dir_fd, 0o700) + +try: + _nofollow_mode = _os.O_RDONLY | _os.O_NONBLOCK | _os.O_NOFOLLOW +except AttributeError: + _nofollow_mode = None + +def _resetperms_at(name, dir_fd, path): + # Same as _resetperms(), but name is resolved relative to the directory + # file descriptor dir_fd. path is only used for os.chflags(), which + # doesn't support dir_fd or file descriptors. + if dir_fd is None: + _resetperms(path) + return + _resetflags(path) + if _os.chmod in _os.supports_follow_symlinks: + _os.chmod(name, 0o700, dir_fd=dir_fd, follow_symlinks=False) + else: + # dir_fd & follow_symlinks is not supported on this platform. + # Try chmod opening the file with O_NOFOLLOW. + if _nofollow_mode is not None: + try: + fd = _os.open(name, _nofollow_mode, dir_fd=dir_fd) + except OSError: + pass + else: + try: + _os.chmod(fd, 0o700) + finally: + _os.close(fd) + return + # If that did not work, we change by name, which is subject to a race + # condition. + stat = _os.lstat(name, dir_fd=dir_fd) + if not _stat.S_ISLNK(stat.st_mode): + _os.chmod(name, 0o700, dir_fd=dir_fd) + # User visible interfaces. @@ -840,28 +894,82 @@ def __init__(self, suffix=None, prefix=None, dir=None, ignore_errors=self._ignore_cleanup_errors) @classmethod - def _rmtree(cls, name, ignore_errors=False): - def onerror(func, path, exc_info): + def _rmtree(cls, name, ignore_errors=False, repeated=False, dir_fd=None, + fullname=None): + if fullname is None: + fullname = name + + def onerror(func, path, exc_info, direntry=None, dir_fd=None): if issubclass(exc_info[0], PermissionError): + if repeated and path == name: + if ignore_errors: + return + raise + + # fullpath is path as seen from the working directory + fullpath = fullname + path[len(name):] + # base is path relative to dir_fd, the directory rmtree() + # reached it through, or the whole path when there is none + if dir_fd is None or not _rmtree_use_dir_fd: + base, dir_fd = path, None + elif direntry is None: + base = path + else: + base = direntry.name + try: if path != name: - _resetperms(_os.path.dirname(path)) - _resetperms(path) + # The parent directory of path is the one referred to + # by dir_fd. + _resetperms_fd(dir_fd, _os.path.dirname(fullpath)) + _resetperms_at(base, dir_fd, fullpath) try: - _os.unlink(path) - # PermissionError is raised on FreeBSD for directories - except (IsADirectoryError, PermissionError): - cls._rmtree(path, ignore_errors=ignore_errors) + _os.unlink(base, dir_fd=dir_fd) + except IsADirectoryError: + cls._rmtree(base, ignore_errors=ignore_errors, + repeated=(path == name), + dir_fd=dir_fd, fullname=fullpath) + except PermissionError: + # The PermissionError handler was originally added for + # FreeBSD in directories, but it seems that it is raised + # on Windows too. + # bpo-43153: Calling _rmtree again may + # raise NotADirectoryError and mask the PermissionError. + # So we must re-raise the current PermissionError if + # path is not a directory. + try: + st = _os.lstat(base, dir_fd=dir_fd) + except OSError: + if ignore_errors: + return + raise + if (_stat.S_ISLNK(st.st_mode) or + not _stat.S_ISDIR(st.st_mode) or + (hasattr(st, 'st_file_attributes') and + st.st_file_attributes & _stat.FILE_ATTRIBUTE_REPARSE_POINT and + st.st_reparse_tag == _stat.IO_REPARSE_TAG_MOUNT_POINT) + ): + if ignore_errors: + return + raise + cls._rmtree(base, ignore_errors=ignore_errors, + repeated=(path == name), + dir_fd=dir_fd, fullname=fullpath) except FileNotFoundError: pass + except OSError: + if ignore_errors: + return + raise elif issubclass(exc_info[0], FileNotFoundError): pass else: if not ignore_errors: raise - _shutil.rmtree(name, onerror=onerror) + _shutil.rmtree(name, onerror=onerror, _dir_fd=dir_fd, + _onerror_kwargs=True) @classmethod def _cleanup(cls, name, warn_message, ignore_errors=False): diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py index d3263c6b68a886..09c1e683669e6b 100644 --- a/Lib/test/test_shutil.py +++ b/Lib/test/test_shutil.py @@ -342,6 +342,37 @@ def test_on_error(self): self.assertEqual(self.errorState, 3, "Expected call to onerror function did not happen.") + @unittest.skipIf(hasattr(os, 'geteuid') and os.geteuid() == 0, + "This test can't be run reliably as root (issue #1076467).") + @unittest.skipUnless(shutil.rmtree.avoids_symlink_attacks, + 'requires the fd based implementation of rmtree()') + def test_on_error_kwargs(self): + os.mkdir(TESTFN) + self.addCleanup(shutil.rmtree, TESTFN) + + child_dir_path = os.path.join(TESTFN, 'b') + child_file_path = os.path.join(child_dir_path, 'a') + os.mkdir(child_dir_path) + os_helper.create_empty_file(child_file_path) + old_child_dir_mode = os.stat(child_dir_path).st_mode + # Make unwritable. + new_mode = stat.S_IREAD|stat.S_IEXEC + os.chmod(child_dir_path, new_mode) + + self.addCleanup(os.chmod, child_dir_path, old_child_dir_mode) + + calls = [] + def onerror(func, path, exc_info, direntry=None, dir_fd=None): + calls.append((func, path, exc_info)) + if func is os.unlink: + self.assertEqual(direntry.name, os.path.basename(path)) + self.assertTrue(os.path.samestat( + os.stat(path), os.stat(direntry.name, dir_fd=dir_fd))) + + shutil.rmtree(TESTFN, onerror=onerror, _onerror_kwargs=True) + self.assertIn((os.unlink, child_file_path), + [(func, path) for func, path, exc_info in calls]) + def check_args_to_onerror(self, func, arg, exc): # test_rmtree_errors deliberately runs rmtree # on a directory that is chmod 500, which will fail. diff --git a/Lib/test/test_tempfile.py b/Lib/test/test_tempfile.py index 6e06bfc7f2f78d..832dbf5206352d 100644 --- a/Lib/test/test_tempfile.py +++ b/Lib/test/test_tempfile.py @@ -12,6 +12,7 @@ import types import weakref import subprocess +import shutil from unittest import mock import unittest @@ -1506,6 +1507,28 @@ def test_explict_cleanup_ignore_errors(self): temp_path.exists(), f"TemporaryDirectory {temp_path!s} exists after cleanup") + @unittest.skipUnless(os.name == "nt", "Only on Windows.") + def test_explicit_cleanup_correct_error(self): + with tempfile.TemporaryDirectory() as working_dir: + temp_dir = self.do_create(dir=working_dir) + with open(os.path.join(temp_dir.name, "example.txt"), 'wb'): + # Previously raised NotADirectoryError on some OSes + # (e.g. Windows). See bpo-43153. + with self.assertRaises(PermissionError): + temp_dir.cleanup() + + @unittest.skipUnless(os.name == "nt", "Only on Windows.") + def test_cleanup_with_used_directory(self): + with tempfile.TemporaryDirectory() as working_dir: + temp_dir = self.do_create(dir=working_dir) + subdir = os.path.join(temp_dir.name, "subdir") + os.mkdir(subdir) + with os_helper.change_cwd(subdir): + # Previously raised RecursionError on some OSes + # (e.g. Windows). See bpo-35144. + with self.assertRaises(PermissionError): + temp_dir.cleanup() + @os_helper.skip_unless_symlink def test_cleanup_with_symlink_to_a_directory(self): # cleanup() should not follow symlinks to directories (issue #12464) @@ -1624,6 +1647,83 @@ def test(target, target_is_directory): new_flags = os.stat(dir1).st_flags self.assertEqual(new_flags, old_flags) + @os_helper.skip_unless_symlink + @unittest.skipIf(hasattr(os, 'geteuid') and os.geteuid() == 0, + 'test needs non-root account') + @unittest.skipUnless(shutil.rmtree.avoids_symlink_attacks, + 'requires the fd based implementation of rmtree()') + def test_cleanup_with_symlink_race(self): + # cleanup() should not operate on files outside of the temporary + # directory when a directory is replaced with a symlink while it + # recovers from a PermissionError (CVE-2026-12345). + with self.do_create(recurse=0) as target: + target_file = os.path.join(target, 'file1') + open(target_file, 'wb').close() + target_mode = os.stat(target_file).st_mode + + d1 = self.do_create(recurse=0) + dir1 = os.path.join(d1.name, 'dir1') + os.mkdir(dir1) + open(os.path.join(dir1, 'file1'), 'wb').close() + # Removing contents of dir1 fails with a PermissionError, and + # dir1 is replaced with a symlink to target at the very moment + # cleanup() starts to recover from that error. + os.chmod(dir1, 0o500) + unlink = os.unlink + def hook(path, *, dir_fd=None): + try: + return unlink(path, dir_fd=dir_fd) + except PermissionError: + if not os.path.islink(dir1): + os.chmod(dir1, 0o700) + os.rename(dir1, dir1 + '_moved') + os.symlink(target, dir1) + raise + try: + with mock.patch('os.unlink', hook): + with contextlib.suppress(OSError): + d1.cleanup() + finally: + if os.path.islink(dir1): + os.unlink(dir1) + os.rename(dir1 + '_moved', dir1) + os.chmod(dir1, 0o700) + d1.cleanup() + + self.assertTrue(os.path.exists(target_file)) + self.assertEqual(os.stat(target_file).st_mode, target_mode) + + @support.subTests('ignore_errors', (True, False)) + def test_parent_mode_preserved(self, ignore_errors): + # Test that cleanup does not touch the parent directory, + # even if that prevents removal. + for mode in range(8): + mode <<= 6 + with self.subTest(mode=format(mode, '03o')): + outer = self.do_create() + with outer: + d = self.do_create(dir=outer.name, dirs=2, files=2, + ignore_cleanup_errors=ignore_errors) + with d: + os.chmod(outer.name, mode) + orig_mode = os.stat(outer.name).st_mode + try: + d.cleanup() + except PermissionError: + if ignore_errors: + raise + self.assertEqual(os.stat(outer.name).st_mode, orig_mode) + outer.cleanup() + self.assertFalse(os.path.exists(outer.name)) + + @unittest.skipUnless(sys.platform.startswith(('linux', 'darwin', 'freebsd')), + 'regression test for supported platforms') + def test_cleanup_safe(self): + """Verify that cleanup uses the safer code path""" + # This is a regression test. Feel free to add exceptions for new + # platforms, but don't forget to update the docs. + self.assertTrue(tempfile._rmtree_use_dir_fd) + @support.cpython_only def test_del_on_collection(self): # A TemporaryDirectory is deleted when garbage collected diff --git a/Misc/NEWS.d/next/Library/2021-12-06-22-10-53.bpo-43153.J7mjSy.rst b/Misc/NEWS.d/next/Library/2021-12-06-22-10-53.bpo-43153.J7mjSy.rst new file mode 100644 index 00000000000000..7800e0a4869adf --- /dev/null +++ b/Misc/NEWS.d/next/Library/2021-12-06-22-10-53.bpo-43153.J7mjSy.rst @@ -0,0 +1,4 @@ +On Windows, ``tempfile.TemporaryDirectory`` previously masked a +``PermissionError`` with ``NotADirectoryError`` during directory cleanup. It +now correctly raises ``PermissionError`` if errors are not ignored. Patch by +Andrei Kulakov and Ken Jin. diff --git a/Misc/NEWS.d/next/Library/2023-12-05-18-57-53.gh-issue-79325.P2vMVK.rst b/Misc/NEWS.d/next/Library/2023-12-05-18-57-53.gh-issue-79325.P2vMVK.rst new file mode 100644 index 00000000000000..f3c32d27b5fe66 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-12-05-18-57-53.gh-issue-79325.P2vMVK.rst @@ -0,0 +1,2 @@ +Fix an infinite recursion error in :func:`tempfile.TemporaryDirectory` +cleanup on Windows. diff --git a/Misc/NEWS.d/next/Security/2026-08-14-11-55-00.gh-issue-157579.Kq3Vt2.rst b/Misc/NEWS.d/next/Security/2026-08-14-11-55-00.gh-issue-157579.Kq3Vt2.rst new file mode 100644 index 00000000000000..46e87575dba983 --- /dev/null +++ b/Misc/NEWS.d/next/Security/2026-08-14-11-55-00.gh-issue-157579.Kq3Vt2.rst @@ -0,0 +1,6 @@ +Fix a race condition in the cleanup of :class:`tempfile.TemporaryDirectory`. +When working around file system permission errors, files are now removed +relative to open directory file descriptors instead of resolving their path +again, so that replacing a directory of the tree with a symbolic link can no +longer make the cleanup delete files outside of the temporary directory. +This addresses :cve:`2026-12345`.