| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
1 parent 202388a commit 06ef3d4
6 files changed
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -205,6 +205,15 @@ The module defines the following user-callable items: | |||
| 205 | 205 | debugging or when you need your cleanup behavior to be conditional based on | |
| 206 | 206 | other logic. | |
| 207 | 207 | ||
| 208 | + .. warning:: | ||
| 209 | + | ||
| 210 | + Cleanup is not robust against the tree being modified while it is removed. | ||
| 211 | + Files outside of the tree may have their permissions and file flags reset. | ||
| 212 | + | ||
| 213 | + On systems where :data:`shutil.rmtree.avoids_symlink_attacks` is | ||
| 214 | + false, manipulating symbolic links during cleanup | ||
| 215 | + may cause files outside of the tree to be removed. | ||
| 216 | + | ||
| 208 | 217 | .. audit-event:: tempfile.mkdtemp fullpath tempfile.TemporaryDirectory | |
| 209 | 218 | ||
| 210 | 219 | .. versionadded:: 3.2 | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -745,6 +745,7 @@ def _rmtree_safe_fd_step(stack, onexc): | |||
| 745 | 745 | # save a call to os.lstat() when walking subdirectories. | |
| 746 | 746 | func, dirfd, path, orig_entry = stack.pop() | |
| 747 | 747 | name = path if orig_entry is None else orig_entry.name | |
| 748 | + parent_fd = None if func is os.close else dirfd | ||
| 748 | 749 | try: | |
| 749 | 750 | if func is os.close: | |
| 750 | 751 | os.close(dirfd) | |
@@ -792,22 +793,23 @@ def _rmtree_safe_fd_step(stack, onexc): | |||
| 792 | 793 | except FileNotFoundError: | |
| 793 | 794 | continue | |
| 794 | 795 | except OSError as err: | |
| 795 | - onexc(os.unlink, fullname, err) | ||
| 796 | + onexc(os.unlink, fullname, err, direntry=entry, dir_fd=topfd) | ||
| 796 | 797 | except FileNotFoundError as err: | |
| 797 | 798 | if orig_entry is None or func is os.close: | |
| 798 | 799 | err.filename = path | |
| 799 | - onexc(func, path, err) | ||
| 800 | + onexc(func, path, err, direntry=orig_entry, dir_fd=parent_fd) | ||
| 800 | 801 | except OSError as err: | |
| 801 | 802 | err.filename = path | |
| 802 | - onexc(func, path, err) | ||
| 803 | + onexc(func, path, err, direntry=orig_entry, dir_fd=parent_fd) | ||
| 803 | 804 | ||
| 804 | 805 | _use_fd_functions = ({os.open, os.stat, os.unlink, os.rmdir} <= | |
| 805 | 806 | os.supports_dir_fd and | |
| 806 | 807 | os.scandir in os.supports_fd and | |
| 807 | 808 | os.stat in os.supports_follow_symlinks) | |
| 808 | 809 | _rmtree_impl = _rmtree_safe_fd if _use_fd_functions else _rmtree_unsafe | |
| 809 | 810 | ||
| 810 | - def rmtree(path, ignore_errors=False, onerror=None, *, onexc=None, dir_fd=None): | ||
| 811 | + def rmtree(path, ignore_errors=False, onerror=None, *, onexc=None, dir_fd=None, | ||
| 812 | + _onexc_kwargs=False): | ||
| 811 | 813 | """Recursively delete a directory tree. | |
| 812 | 814 | ||
| 813 | 815 | If dir_fd is not None, it should be a file descriptor open to a directory; | |
@@ -830,24 +832,29 @@ def rmtree(path, ignore_errors=False, onerror=None, *, onexc=None, dir_fd=None): | |||
| 830 | 832 | ||
| 831 | 833 | sys.audit("shutil.rmtree", path, dir_fd) | |
| 832 | 834 | if ignore_errors: | |
| 833 | - def onexc(*args): | ||
| 835 | + def onexc(*args, **kwargs): | ||
| 834 | 836 | pass | |
| 835 | 837 | elif onerror is None and onexc is None: | |
| 836 | - def onexc(*args): | ||
| 838 | + def onexc(*args, **kwargs): | ||
| 837 | 839 | raise | |
| 838 | 840 | elif onexc is None: | |
| 839 | 841 | if onerror is None: | |
| 840 | - def onexc(*args): | ||
| 842 | + def onexc(*args, **kwargs): | ||
| 841 | 843 | raise | |
| 842 | 844 | else: | |
| 843 | 845 | # delegate to onerror | |
| 844 | - def onexc(*args): | ||
| 846 | + def onexc(*args, **kwargs): | ||
| 845 | 847 | func, path, exc = args | |
| 846 | 848 | if exc is None: | |
| 847 | 849 | exc_info = None, None, None | |
| 848 | 850 | else: | |
| 849 | 851 | exc_info = type(exc), exc, exc.__traceback__ | |
| 850 | 852 | return onerror(func, path, exc_info) | |
| 853 | + elif not _onexc_kwargs: | ||
| 854 | + # Only the internal caller in tempfile asks for the extra arguments. | ||
| 855 | + _onexc = onexc | ||
| 856 | + def onexc(func, path, err, **kwargs): | ||
| 857 | + return _onexc(func, path, err) | ||
| 851 | 858 | ||
| 852 | 859 | _rmtree_impl(path, dir_fd, onexc) | |
| 853 | 860 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -43,6 +43,7 @@ | |||
| 43 | 43 | import shutil as _shutil | |
| 44 | 44 | import errno as _errno | |
| 45 | 45 | from random import Random as _Random | |
| 46 | + import stat as _stat | ||
| 46 | 47 | import sys as _sys | |
| 47 | 48 | import types as _types | |
| 48 | 49 | import weakref as _weakref | |
@@ -273,15 +274,68 @@ def _dont_follow_symlinks(func, path, *args): | |||
| 273 | 274 | elif not _os.path.islink(path): | |
| 274 | 275 | func(path, *args) | |
| 275 | 276 | ||
| 276 | - def _resetperms(path): | ||
| 277 | + def _resetflags(path): | ||
| 277 | 278 | try: | |
| 278 | 279 | chflags = _os.chflags | |
| 279 | 280 | except AttributeError: | |
| 280 | 281 | pass | |
| 281 | 282 | else: | |
| 282 | 283 | _dont_follow_symlinks(chflags, path, 0) | |
| 284 | + | ||
| 285 | + def _resetperms(path): | ||
| 286 | + _resetflags(path) | ||
| 283 | 287 | _dont_follow_symlinks(_os.chmod, path, 0o700) | |
| 284 | 288 | ||
| 289 | + # True if TemporaryDirectory._rmtree() can work relative to open directories | ||
| 290 | + # instead of resolving paths again. | ||
| 291 | + _rmtree_use_dir_fd = ( | ||
| 292 | + {_os.chmod, _os.unlink, _os.lstat} <= _os.supports_dir_fd | ||
| 293 | + and _os.chmod in _os.supports_fd | ||
| 294 | + ) | ||
| 295 | + | ||
| 296 | + def _resetperms_fd(dir_fd, path): | ||
| 297 | + # Same as _resetperms(), but for the directory referred to by dir_fd. | ||
| 298 | + if dir_fd is None: | ||
| 299 | + _resetperms(path) | ||
| 300 | + return | ||
| 301 | + _resetflags(path) | ||
| 302 | + _os.chmod(dir_fd, 0o700) | ||
| 303 | + | ||
| 304 | + try: | ||
| 305 | + _nofollow_mode = _os.O_RDONLY | _os.O_NONBLOCK | _os.O_NOFOLLOW | ||
| 306 | + except AttributeError: | ||
| 307 | + _nofollow_mode = None | ||
| 308 | + | ||
| 309 | + def _resetperms_at(name, dir_fd, path): | ||
| 310 | + # Same as _resetperms(), but name is resolved relative to the directory | ||
| 311 | + # file descriptor dir_fd. path is only used for os.chflags(), which | ||
| 312 | + # doesn't support dir_fd or file descriptors. | ||
| 313 | + if dir_fd is None: | ||
| 314 | + _resetperms(path) | ||
| 315 | + return | ||
| 316 | + _resetflags(path) | ||
| 317 | + if _os.chmod in _os.supports_follow_symlinks: | ||
| 318 | + _os.chmod(name, 0o700, dir_fd=dir_fd, follow_symlinks=False) | ||
| 319 | + else: | ||
| 320 | + # dir_fd & follow_symlinks is not supported on this platform. | ||
| 321 | + # Try chmod opening the file with O_NOFOLLOW. | ||
| 322 | + if _nofollow_mode is not None: | ||
| 323 | + try: | ||
| 324 | + fd = _os.open(name, _nofollow_mode, dir_fd=dir_fd) | ||
| 325 | + except OSError: | ||
| 326 | + pass | ||
| 327 | + else: | ||
| 328 | + try: | ||
| 329 | + _os.chmod(fd, 0o700) | ||
| 330 | + finally: | ||
| 331 | + _os.close(fd) | ||
| 332 | + return | ||
| 333 | + # If that did not work, we change by name, which is subject to a race | ||
| 334 | + # condition. | ||
| 335 | + stat = _os.lstat(name, dir_fd=dir_fd) | ||
| 336 | + if not _stat.S_ISLNK(stat.st_mode): | ||
| 337 | + _os.chmod(name, 0o700, dir_fd=dir_fd) | ||
| 338 | + | ||
| 285 | 339 | ||
| 286 | 340 | # User visible interfaces. | |
| 287 | 341 | ||
@@ -913,24 +967,43 @@ def __init__(self, suffix=None, prefix=None, dir=None, | |||
| 913 | 967 | ignore_errors=self._ignore_cleanup_errors, delete=self._delete) | |
| 914 | 968 | ||
| 915 | 969 | @classmethod | |
| 916 | - def _rmtree(cls, name, ignore_errors=False, repeated=False): | ||
| 917 | - def onexc(func, path, exc): | ||
| 970 | + def _rmtree(cls, name, ignore_errors=False, repeated=False, dir_fd=None, | ||
| 971 | + fullname=None): | ||
| 972 | + if fullname is None: | ||
| 973 | + fullname = name | ||
| 974 | + | ||
| 975 | + def onexc(func, path, exc, direntry=None, dir_fd=None): | ||
| 918 | 976 | # On DragonFly BSD, UF_NOUNLINK removal fails with EISDIR, not EPERM. | |
| 919 | 977 | if isinstance(exc, (PermissionError, IsADirectoryError)): | |
| 920 | 978 | if repeated and path == name: | |
| 921 | 979 | if ignore_errors: | |
| 922 | 980 | return | |
| 923 | 981 | raise | |
| 924 | 982 | ||
| 983 | + # fullpath is path as seen from the working directory | ||
| 984 | + fullpath = fullname + path[len(name):] | ||
| 985 | + # base is path relative to dir_fd, the directory rmtree() | ||
| 986 | + # reached it through, or the whole path when there is none | ||
| 987 | + if dir_fd is None or not _rmtree_use_dir_fd: | ||
| 988 | + base, dir_fd = path, None | ||
| 989 | + elif direntry is None: | ||
| 990 | + base = path | ||
| 991 | + else: | ||
| 992 | + base = direntry.name | ||
| 993 | + | ||
| 925 | 994 | try: | |
| 926 | 995 | if path != name: | |
| 927 | - _resetperms(_os.path.dirname(path)) | ||
| 928 | - _resetperms(path) | ||
| 996 | + # The parent directory of path is the one referred to | ||
| 997 | + # by dir_fd. | ||
| 998 | + _resetperms_fd(dir_fd, _os.path.dirname(fullpath)) | ||
| 999 | + _resetperms_at(base, dir_fd, fullpath) | ||
| 929 | 1000 | ||
| 930 | 1001 | try: | |
| 931 | - _os.unlink(path) | ||
| 1002 | + _os.unlink(base, dir_fd=dir_fd) | ||
| 932 | 1003 | except IsADirectoryError: | |
| 933 | - cls._rmtree(path, ignore_errors=ignore_errors) | ||
| 1004 | + cls._rmtree(base, ignore_errors=ignore_errors, | ||
| 1005 | + repeated=(path == name), | ||
| 1006 | + dir_fd=dir_fd, fullname=fullpath) | ||
| 934 | 1007 | except PermissionError: | |
| 935 | 1008 | # The PermissionError handler was originally added for | |
| 936 | 1009 | # FreeBSD in directories, but it seems that it is raised | |
@@ -939,21 +1012,27 @@ def onexc(func, path, exc): | |||
| 939 | 1012 | # raise NotADirectoryError and mask the PermissionError. | |
| 940 | 1013 | # So we must re-raise the current PermissionError if | |
| 941 | 1014 | # path is not a directory. | |
| 942 | - if not _os.path.isdir(path) or _os.path.isjunction(path): | ||
| 1015 | + if (not _os.path.isdir(fullpath) | ||
| 1016 | + or _os.path.isjunction(fullpath)): | ||
| 943 | 1017 | if ignore_errors: | |
| 944 | 1018 | return | |
| 945 | 1019 | raise | |
| 946 | - cls._rmtree(path, ignore_errors=ignore_errors, | ||
| 947 | - repeated=(path == name)) | ||
| 1020 | + cls._rmtree(base, ignore_errors=ignore_errors, | ||
| 1021 | + repeated=(path == name), | ||
| 1022 | + dir_fd=dir_fd, fullname=fullpath) | ||
| 948 | 1023 | except FileNotFoundError: | |
| 949 | 1024 | pass | |
| 1025 | + except OSError: | ||
| 1026 | + if ignore_errors: | ||
| 1027 | + return | ||
| 1028 | + raise | ||
| 950 | 1029 | elif isinstance(exc, FileNotFoundError): | |
| 951 | 1030 | pass | |
| 952 | 1031 | else: | |
| 953 | 1032 | if not ignore_errors: | |
| 954 | 1033 | raise | |
| 955 | 1034 | ||
| 956 | - _shutil.rmtree(name, onexc=onexc) | ||
| 1035 | + _shutil.rmtree(name, onexc=onexc, dir_fd=dir_fd, _onexc_kwargs=True) | ||
| 957 | 1036 | ||
| 958 | 1037 | @classmethod | |
| 959 | 1038 | def _cleanup(cls, name, warn_message, ignore_errors=False, delete=True): | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -493,6 +493,37 @@ def check_args_to_onexc(self, func, arg, exc): | |||
| 493 | 493 | self.assertTrue(isinstance(exc, OSError)) | |
| 494 | 494 | self.errorState = 3 | |
| 495 | 495 | ||
| 496 | + @os_helper.skip_if_dac_override | ||
| 497 | + @os_helper.skip_unless_working_chmod | ||
| 498 | + @unittest.skipUnless(shutil.rmtree.avoids_symlink_attacks, | ||
| 499 | + 'requires the fd based implementation of rmtree()') | ||
| 500 | + def test_on_exc_kwargs(self): | ||
| 501 | + os.mkdir(TESTFN) | ||
| 502 | + self.addCleanup(shutil.rmtree, TESTFN) | ||
| 503 | + | ||
| 504 | + child_dir_path = os.path.join(TESTFN, 'b') | ||
| 505 | + child_file_path = os.path.join(child_dir_path, 'a') | ||
| 506 | + os.mkdir(child_dir_path) | ||
| 507 | + os_helper.create_empty_file(child_file_path) | ||
| 508 | + old_child_dir_mode = os.stat(child_dir_path).st_mode | ||
| 509 | + # Make unwritable. | ||
| 510 | + new_mode = stat.S_IREAD|stat.S_IEXEC | ||
| 511 | + os.chmod(child_dir_path, new_mode) | ||
| 512 | + | ||
| 513 | + self.addCleanup(os.chmod, child_dir_path, old_child_dir_mode) | ||
| 514 | + | ||
| 515 | + calls = [] | ||
| 516 | + def onexc(func, path, err, direntry=None, dir_fd=None): | ||
| 517 | + calls.append((func, path, err)) | ||
| 518 | + if func is os.unlink: | ||
| 519 | + self.assertEqual(direntry.name, os.path.basename(path)) | ||
| 520 | + self.assertTrue(os.path.samestat( | ||
| 521 | + os.stat(path), os.stat(direntry.name, dir_fd=dir_fd))) | ||
| 522 | + | ||
| 523 | + shutil.rmtree(TESTFN, onexc=onexc, _onexc_kwargs=True) | ||
| 524 | + self.assertIn((os.unlink, child_file_path), | ||
| 525 | + [(func, path) for func, path, err in calls]) | ||
| 526 | + | ||
| 496 | 527 | @unittest.skipIf(sys.platform[:6] == 'cygwin', | |
| 497 | 528 | "This test can't be run on Cygwin (issue #1071513).") | |
| 498 | 529 | @os_helper.skip_if_dac_override | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -14,6 +14,7 @@ | |||
| 14 | 14 | import gc | |
| 15 | 15 | import shutil | |
| 16 | 16 | import subprocess | |
| 17 | + import sysconfig | ||
| 17 | 18 | from unittest import mock | |
| 18 | 19 | ||
| 19 | 20 | import unittest | |
@@ -1847,6 +1848,54 @@ def test(target, target_is_directory): | |||
| 1847 | 1848 | new_flags = os.stat(dir1).st_flags | |
| 1848 | 1849 | self.assertEqual(new_flags, old_flags) | |
| 1849 | 1850 | ||
| 1851 | + @os_helper.skip_unless_symlink | ||
| 1852 | + @os_helper.skip_unless_working_chmod | ||
| 1853 | + @support.requires_non_root_user | ||
| 1854 | + @unittest.skipIf(support.is_emscripten, 'Fails due to Emscripten bug:' | ||
| 1855 | + 'emscripten-core/emscripten#27761') | ||
| 1856 | + @unittest.skipUnless(shutil.rmtree.avoids_symlink_attacks, | ||
| 1857 | + 'requires the fd based implementation of rmtree()') | ||
| 1858 | + def test_cleanup_with_symlink_race(self): | ||
| 1859 | + # cleanup() should not operate on files outside of the temporary | ||
| 1860 | + # directory when a directory is replaced with a symlink while it | ||
| 1861 | + # recovers from a PermissionError (CVE-2026-12345). | ||
| 1862 | + with self.do_create(recurse=0) as target: | ||
| 1863 | + target_file = os.path.join(target, 'file1') | ||
| 1864 | + open(target_file, 'wb').close() | ||
| 1865 | + target_mode = os.stat(target_file).st_mode | ||
| 1866 | + | ||
| 1867 | + d1 = self.do_create(recurse=0) | ||
| 1868 | + dir1 = os.path.join(d1.name, 'dir1') | ||
| 1869 | + os.mkdir(dir1) | ||
| 1870 | + open(os.path.join(dir1, 'file1'), 'wb').close() | ||
| 1871 | + # Removing contents of dir1 fails with a PermissionError, and | ||
| 1872 | + # dir1 is replaced with a symlink to target at the very moment | ||
| 1873 | + # cleanup() starts to recover from that error. | ||
| 1874 | + os.chmod(dir1, 0o500) | ||
| 1875 | + unlink = os.unlink | ||
| 1876 | + def hook(path, *, dir_fd=None): | ||
| 1877 | + try: | ||
| 1878 | + return unlink(path, dir_fd=dir_fd) | ||
| 1879 | + except PermissionError: | ||
| 1880 | + if not os.path.islink(dir1): | ||
| 1881 | + os.chmod(dir1, 0o700) | ||
| 1882 | + os.rename(dir1, dir1 + '_moved') | ||
| 1883 | + os.symlink(target, dir1) | ||
| 1884 | + raise | ||
| 1885 | + try: | ||
| 1886 | + with mock.patch('os.unlink', hook): | ||
| 1887 | + with contextlib.suppress(OSError): | ||
| 1888 | + d1.cleanup() | ||
| 1889 | + finally: | ||
| 1890 | + if os.path.islink(dir1): | ||
| 1891 | + os.unlink(dir1) | ||
| 1892 | + os.rename(dir1 + '_moved', dir1) | ||
| 1893 | + os.chmod(dir1, 0o700) | ||
| 1894 | + d1.cleanup() | ||
| 1895 | + | ||
| 1896 | + self.assertTrue(os.path.exists(target_file)) | ||
| 1897 | + self.assertEqual(os.stat(target_file).st_mode, target_mode) | ||
| 1898 | + | ||
| 1850 | 1899 | @support.cpython_only | |
| 1851 | 1900 | def test_del_on_collection(self): | |
| 1852 | 1901 | # A TemporaryDirectory is deleted when garbage collected | |
@@ -2019,6 +2068,29 @@ def test_modes(self): | |||
| 2019 | 2068 | d.cleanup() | |
| 2020 | 2069 | self.assertFalse(os.path.exists(d.name)) | |
| 2021 | 2070 | ||
| 2071 | + @support.subTests('ignore_errors', (True, False)) | ||
| 2072 | + def test_parent_mode_preserved(self, ignore_errors): | ||
| 2073 | + # Test that cleanup does not touch the parent directory, | ||
| 2074 | + # even if that prevents removal. | ||
| 2075 | + for mode in range(8): | ||
| 2076 | + mode <<= 6 | ||
| 2077 | + with self.subTest(mode=format(mode, '03o')): | ||
| 2078 | + outer = self.do_create() | ||
| 2079 | + with outer: | ||
| 2080 | + d = self.do_create(dir=outer.name, dirs=2, files=2, | ||
| 2081 | + ignore_cleanup_errors=ignore_errors) | ||
| 2082 | + with d: | ||
| 2083 | + os.chmod(outer.name, mode) | ||
| 2084 | + orig_mode = os.stat(outer.name).st_mode | ||
| 2085 | + try: | ||
| 2086 | + d.cleanup() | ||
| 2087 | + except PermissionError: | ||
| 2088 | + if ignore_errors: | ||
| 2089 | + raise | ||
| 2090 | + self.assertEqual(os.stat(outer.name).st_mode, orig_mode) | ||
| 2091 | + outer.cleanup() | ||
| 2092 | + self.assertFalse(os.path.exists(outer.name)) | ||
| 2093 | + | ||
| 2022 | 2094 | def check_flags(self, flags): | |
| 2023 | 2095 | # skip the test if these flags are not supported (ex: FreeBSD 13) | |
| 2024 | 2096 | filename = os_helper.TESTFN | |
@@ -2056,5 +2128,17 @@ def test_delete_false(self): | |||
| 2056 | 2128 | self.assertTrue(os.path.exists(working_dir)) | |
| 2057 | 2129 | shutil.rmtree(working_dir) | |
| 2058 | 2130 | ||
| 2131 | + @unittest.skipUnless( | ||
| 2132 | + sysconfig.get_config_var('PY_SUPPORT_TIER') | ||
| 2133 | + and sysconfig.get_config_var('PY_SUPPORT_TIER') <= 3, | ||
| 2134 | + 'regression test for supported platforms') | ||
| 2135 | + @unittest.skipIf(support.MS_WINDOWS, 'dirfd not used on Windows') | ||
| 2136 | + @unittest.skipIf(support.is_wasi, 'WASI has no chmod') | ||
| 2137 | + def test_cleanup_safe(self): | ||
| 2138 | + """Verify that cleanup uses the safer code path""" | ||
| 2139 | + # This is a regression test. Feel free to add exceptions for new | ||
| 2140 | + # platforms, but don't forget to update the docs. | ||
| 2141 | + self.assertTrue(tempfile._rmtree_use_dir_fd) | ||
| 2142 | + | ||
| 2059 | 2143 | if __name__ == "__main__": | |
| 2060 | 2144 | unittest.main() | |
| Back | FazBrowse Home | New Git URL |
0 commit comments