From f353509fe2a3f44349c59a735d42679d59bd09fd Mon Sep 17 00:00:00 2001 From: "Miss Islington (bot)" <31488909+miss-islington@users.noreply.github.com> Date: Tue, 5 Dec 2023 14:44:49 +0100 Subject: [PATCH 1/5] [3.11] bpo-43153: Don't mask `PermissionError` with `NotADirectoryError` during tempdirectory cleanup (GH-29940) (GH-112754) (cherry picked from commit 8cdfee1bb902fd1e38d79170b751ef13a0907262) Co-authored-by: Ken Jin Co-authored-by: andrei kulakov Co-authored-by: Serhiy Storchaka (cherry picked from commit 23920a0843f48c4f4be4a31b56ee2e44c058099b) (cherry picked from commit e3888c05418b892dcfd38d75b7d98c0b73a524da) 3.9.25.x: 3.9's TemporaryDirectory has no ignore_cleanup_errors (added in 3.10), so _rmtree() gains a private ignore_errors parameter that is always False here; it keeps the code in the 3.10/3.11 shape for the follow-up fixes. The ignore_cleanup_errors test is not ported. --- Lib/tempfile.py | 35 ++++++++++++++++--- Lib/test/test_tempfile.py | 10 ++++++ .../2021-12-06-22-10-53.bpo-43153.J7mjSy.rst | 4 +++ 3 files changed, 44 insertions(+), 5 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2021-12-06-22-10-53.bpo-43153.J7mjSy.rst diff --git a/Lib/tempfile.py b/Lib/tempfile.py index 59a628a17446855..46274081110a649 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 @@ -802,7 +803,7 @@ def __init__(self, suffix=None, prefix=None, dir=None): warn_message="Implicitly cleaning up {!r}".format(self)) @classmethod - def _rmtree(cls, name): + def _rmtree(cls, name, ignore_errors=False): def onerror(func, path, exc_info): if issubclass(exc_info[0], PermissionError): try: @@ -812,15 +813,39 @@ def onerror(func, path, exc_info): try: _os.unlink(path) - # PermissionError is raised on FreeBSD for directories - except (IsADirectoryError, PermissionError): - cls._rmtree(path) + except IsADirectoryError: + cls._rmtree(path, ignore_errors=ignore_errors) + 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(path) + 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(path, ignore_errors=ignore_errors) except FileNotFoundError: pass elif issubclass(exc_info[0], FileNotFoundError): pass else: - raise + if not ignore_errors: + raise _shutil.rmtree(name, onerror=onerror) diff --git a/Lib/test/test_tempfile.py b/Lib/test/test_tempfile.py index 8d13c22f2ee8a1f..dfa5826e82bf305 100644 --- a/Lib/test/test_tempfile.py +++ b/Lib/test/test_tempfile.py @@ -1401,6 +1401,16 @@ def test_explicit_cleanup(self): finally: os.rmdir(dir) + @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() + @support.skip_unless_symlink def test_cleanup_with_symlink_to_a_directory(self): # cleanup() should not follow symlinks to directories (issue #12464) 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 000000000000000..7800e0a4869adff --- /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. From 7de735bee19b63fa3677b870acd876dd6912e85e Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Thu, 7 Dec 2023 19:46:30 +0200 Subject: [PATCH 2/5] [3.11] gh-79325: Fix recursion error in TemporaryDirectory cleanup on Windows (GH-112762) (GH-112848) (cherry picked from commit b2923a61a10dc2717f4662b590cc9f6d181c6983) (cherry picked from commit a18201bff1c5d0e29e6f04e7356896f41b3ed9ac) (cherry picked from commit ffb9ae4f99a9f1a2e5bb9319830f6eacb8c7a5db) 3.9.25.x: test uses support.change_cwd (3.9 has no test.support.os_helper). --- Lib/tempfile.py | 10 ++++++++-- Lib/test/test_tempfile.py | 12 ++++++++++++ .../2023-12-05-18-57-53.gh-issue-79325.P2vMVK.rst | 2 ++ 3 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2023-12-05-18-57-53.gh-issue-79325.P2vMVK.rst diff --git a/Lib/tempfile.py b/Lib/tempfile.py index 46274081110a649..054e10df847589f 100644 --- a/Lib/tempfile.py +++ b/Lib/tempfile.py @@ -803,9 +803,14 @@ def __init__(self, suffix=None, prefix=None, dir=None): warn_message="Implicitly cleaning up {!r}".format(self)) @classmethod - def _rmtree(cls, name, ignore_errors=False): + def _rmtree(cls, name, ignore_errors=False, repeated=False): def onerror(func, path, exc_info): if issubclass(exc_info[0], PermissionError): + if repeated and path == name: + if ignore_errors: + return + raise + try: if path != name: _resetperms(_os.path.dirname(path)) @@ -838,7 +843,8 @@ def onerror(func, path, exc_info): if ignore_errors: return raise - cls._rmtree(path, ignore_errors=ignore_errors) + cls._rmtree(path, ignore_errors=ignore_errors, + repeated=(path == name)) except FileNotFoundError: pass elif issubclass(exc_info[0], FileNotFoundError): diff --git a/Lib/test/test_tempfile.py b/Lib/test/test_tempfile.py index dfa5826e82bf305..8e248ddecef515e 100644 --- a/Lib/test/test_tempfile.py +++ b/Lib/test/test_tempfile.py @@ -1411,6 +1411,18 @@ def test_explicit_cleanup_correct_error(self): 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 support.change_cwd(subdir): + # Previously raised RecursionError on some OSes + # (e.g. Windows). See bpo-35144. + with self.assertRaises(PermissionError): + temp_dir.cleanup() + @support.skip_unless_symlink def test_cleanup_with_symlink_to_a_directory(self): # cleanup() should not follow symlinks to directories (issue #12464) 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 000000000000000..f3c32d27b5fe661 --- /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. From 9490a2cedc86251470bb782600d464f496d5d65e Mon Sep 17 00:00:00 2001 From: icanhasmath Date: Tue, 27 Oct 2026 22:13:03 -0500 Subject: [PATCH 3/5] [3.9] gh-157579: Fix race condition in the cleanup of tempfile.TemporaryDirectory (CVE-2026-12345) When TemporaryDirectory cleanup recovers from a PermissionError it resets permissions and retries removal. It did that by resolving paths again, so an attacker who could modify the tree during cleanup could replace a directory with a symlink and make cleanup delete files, or reset their permissions and flags, outside of the temporary directory. Files are now handled relative to the open directory file descriptors that the fd-based shutil.rmtree() walk already holds. Port of the upstream fix (GH-157580, GH-158430), based on the 3.12 backport (#159074), plus gh-134993 (os.lstat in os.supports_dir_fd). 3.10.22.x: the 3.12 fix relies on 3.12's shutil.rmtree() API (onexc, a public dir_fd parameter, and the _onexc_kwargs switch that passes direntry/dir_fd to the callback). 3.10 only has onerror and a recursive _rmtree_safe_fd(), so this adds the private keyword arguments _dir_fd and _onerror_kwargs to rmtree() instead and threads the parent directory fd and os.DirEntry through _rmtree_safe_fd(). With _onerror_kwargs left false, existing onerror callbacks get the same three positional arguments as before. TemporaryDirectory._rmtree() is ported onto the 3.11 shape of the code (bpo-43153 and gh-79325 are picked first), using os.lstat(base, dir_fd=...) where 3.12 uses os.path.isdir()/isjunction() on the full path. Tests are adapted: onerror instead of onexc, local root and platform skips instead of support.requires_non_root_user and PY_SUPPORT_TIER. (cherry picked from commit c39b52d00b42200aaadff679e2fe3f1006d57c58) (cherry picked from commit 5c20517a4fc56683efe63a7751020db9573f538d) Co-Authored-By: Claude Opus 5.5 (cherry picked from commit 776b466bd1217fc7ce59d6e5d4729ba80d4c7a19) 3.9.25.x: tests use test.support instead of test.support.os_helper, and test_parent_mode_preserved only covers the default (no ignore_errors) case because 3.9's TemporaryDirectory has no ignore_cleanup_errors. --- Lib/os.py | 1 + Lib/shutil.py | 61 +++++++---- Lib/tempfile.py | 100 ++++++++++++++++-- Lib/test/test_shutil.py | 31 ++++++ Lib/test/test_tempfile.py | 75 +++++++++++++ ...-08-14-11-55-00.gh-issue-157579.Kq3Vt2.rst | 6 ++ 6 files changed, 243 insertions(+), 31 deletions(-) create mode 100644 Misc/NEWS.d/next/Security/2026-08-14-11-55-00.gh-issue-157579.Kq3Vt2.rst diff --git a/Lib/os.py b/Lib/os.py index b794159f86c33ef..f7d757279f0b9af 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 a55916717974c8f..649042d6706a091 100644 --- a/Lib/shutil.py +++ b/Lib/shutil.py @@ -632,14 +632,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) @@ -653,24 +656,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 @@ -679,7 +685,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) @@ -687,14 +694,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 @@ -705,13 +714,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): @@ -719,31 +739,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 054e10df847589f..2dce74ade8dea4a 100644 --- a/Lib/tempfile.py +++ b/Lib/tempfile.py @@ -276,15 +276,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. @@ -803,23 +856,42 @@ def __init__(self, suffix=None, prefix=None, dir=None): warn_message="Implicitly cleaning up {!r}".format(self)) @classmethod - def _rmtree(cls, name, ignore_errors=False, repeated=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) + _os.unlink(base, dir_fd=dir_fd) except IsADirectoryError: - cls._rmtree(path, ignore_errors=ignore_errors) + 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 @@ -829,7 +901,7 @@ def onerror(func, path, exc_info): # So we must re-raise the current PermissionError if # path is not a directory. try: - st = _os.lstat(path) + st = _os.lstat(base, dir_fd=dir_fd) except OSError: if ignore_errors: return @@ -843,17 +915,23 @@ def onerror(func, path, exc_info): if ignore_errors: return raise - cls._rmtree(path, ignore_errors=ignore_errors, - repeated=(path == name)) + 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): diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py index fc6c854a91d1223..acca30545bb556b 100644 --- a/Lib/test/test_shutil.py +++ b/Lib/test/test_shutil.py @@ -337,6 +337,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) + support.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 8e248ddecef515e..7cc6f989fa9bb01 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 @@ -1541,6 +1542,80 @@ def test(target, target_is_directory): new_flags = os.stat(dir1).st_flags self.assertEqual(new_flags, old_flags) + @support.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) + + def test_parent_mode_preserved(self): + # 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) + with d: + os.chmod(outer.name, mode) + orig_mode = os.stat(outer.name).st_mode + try: + d.cleanup() + except PermissionError: + pass + 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/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 000000000000000..46e87575dba9839 --- /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`. From 992acc8d40a76b21292734fc8c99bff15e372d50 Mon Sep 17 00:00:00 2001 From: icanhasmath Date: Tue, 27 Oct 2026 22:15:14 -0500 Subject: [PATCH 4/5] Update version to 3.9.25.2 and add release notes ActiveState security release of the end-of-life 3.9 line: the gh-157579 tempfile.TemporaryDirectory cleanup race fix (CVE-2026-12345), ported from 3.12, with its prerequisites bpo-43153 and gh-79325. Co-Authored-By: Claude Opus 5.5 --- Doc/whatsnew/3.9.rst | 21 +++++++++++++++++++++ Include/patchlevel.h | 2 +- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/Doc/whatsnew/3.9.rst b/Doc/whatsnew/3.9.rst index 453ddc80cda58bf..a9e747a8c37e946 100644 --- a/Doc/whatsnew/3.9.rst +++ b/Doc/whatsnew/3.9.rst @@ -1826,3 +1826,24 @@ Other Not included: the expat amplification-tuning API from :gh:`90949`. The protection itself (CVE-2025-59375) is built into libexpat 2.7.2 and later and is on by default, so 3.9 gets it from the bundled libexpat 2.9.0. + + +Notable changes in 3.9.25.2 +=========================== + +This is an ActiveState security release of the 3.9 line, on top of +3.9.25.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 7d80b1a1c18afce..988aba035a6fa8b 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.9.25.1" +#define PY_VERSION "3.9.25.2" /*--end constants--*/ /* Version as a single 4-byte hex number, e.g. 0x010502B2 == 1.5.2b2. From 89e9acb95279c031ea096c0b239e6c90c6710900 Mon Sep 17 00:00:00 2001 From: icanhasmath Date: Wed, 28 Oct 2026 12:25:37 -0500 Subject: [PATCH 5/5] Fix the temporary-directory leak in test_cleanup_with_symlink_race (3.9) Review follow-up. 3.9's TemporaryDirectory.cleanup() only removes the tree if self._finalizer.detach() succeeds. The test's first cleanup() detaches the finalizer even though it raises, so the second cleanup() in the finally block did nothing and left /dir1 behind; regrtest reported test_tempfile as "env changed". Remove the tree with d1._rmtree() instead. (3.10's cleanup() also retries while the directory still exists, so #89 is unaffected.) Co-Authored-By: Claude Opus 5.5 --- Lib/test/test_tempfile.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_tempfile.py b/Lib/test/test_tempfile.py index 7cc6f989fa9bb01..55f6f716d80674b 100644 --- a/Lib/test/test_tempfile.py +++ b/Lib/test/test_tempfile.py @@ -1583,7 +1583,9 @@ def hook(path, *, dir_fd=None): os.unlink(dir1) os.rename(dir1 + '_moved', dir1) os.chmod(dir1, 0o700) - d1.cleanup() + # The failed cleanup() above already detached the finalizer, + # and 3.9's cleanup() does nothing once it is detached. + d1._rmtree(d1.name) self.assertTrue(os.path.exists(target_file)) self.assertEqual(os.stat(target_file).st_mode, target_mode)