diff --git a/Doc/library/shutil.rst b/Doc/library/shutil.rst --- a/Doc/library/shutil.rst +++ b/Doc/library/shutil.rst @@ -177,14 +177,27 @@ handled by calling a handler specified by *onerror* or, if that is omitted, they raise an exception. + .. warning:: + + The default :func:`rmtree` function is susceptible to a symlink attack: + given proper timing and circumstances, attackers can use it to delete + files they wouldn't be able to access otherwise. Thus -- on platforms + that support the necessary :func:`os.fwalk` and :func:`os.unlinkat` + functions -- a safe version of :func:`rmtree` is used, which isn't + vulnerable. + If *onerror* is provided, it must be a callable that accepts three - parameters: *function*, *path*, and *excinfo*. The first parameter, - *function*, is the function which raised the exception; it will be - :func:`os.path.islink`, :func:`os.listdir`, :func:`os.remove` or - :func:`os.rmdir`. The second parameter, *path*, will be the path name passed - to *function*. The third parameter, *excinfo*, will be the exception - information return by :func:`sys.exc_info`. Exceptions raised by *onerror* - will not be caught. + parameters: *function*, *path*, and *excinfo*. + + The first parameter, *function*, is the function which raised the exception; + it depends on the platform and implementation. The second parameter, + *path*, will be the path name passed to *function*. The third parameter, + *excinfo*, will be the exception information returned by + :func:`sys.exc_info`. Exceptions raised by *onerror* will not be caught. + + .. versionchanged:: 3.3 + Added a safe version that is used automatically if platform supports + :func:`os.fwalk` and :func:`os.unlinkat`. .. function:: move(src, dst) diff --git a/Lib/shutil.py b/Lib/shutil.py --- a/Lib/shutil.py +++ b/Lib/shutil.py @@ -323,15 +323,67 @@ if errors: raise Error(errors) -def rmtree(path, ignore_errors=False, onerror=None): +def _safe_fwalk_rmtree(path, ignore_errors=False, onerror=None): """Recursively delete a directory tree. - If ignore_errors is set, errors are ignored; otherwise, if onerror - is set, it is called to handle the error with arguments (func, - path, exc_info) where func is os.listdir, os.remove, or os.rmdir; - path is the argument to that function that caused it to fail; and - exc_info is a tuple returned by sys.exc_info(). If ignore_errors - is false and onerror is None, an exception is raised. + If ignore_errors is set, errors are ignored; otherwise, if onerror is set, + it is called to handle the error with arguments (func, path, exc_info) + where func is os.path.islink, os.fwalk, os.unlinkat, or os.rmdir; path is + the argument to that function that caused it to fail; and exc_info is + a tuple returned by sys.exc_info(). If ignore_errors is false and onerror + is None, an exception is raised. + + Uses safe functions to avoid symlink attacks. + + """ + if ignore_errors: + def onerror(*args): + pass + elif onerror is None: + def onerror(*args): + raise + try: + if os.path.islink(path): + # 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()) + # can't continue even if onerror hook returns + return + + try: + for root, dirs, files, rootfd in os.fwalk(path, topdown=False): + for name in files: + try: + os.unlinkat(rootfd, name) + except os.error: + onerror(os.unlinkat, + os.path.join(path, name), + sys.exc_info()) + for name in dirs: + try: + os.unlinkat(rootfd, name, os.AT_REMOVEDIR) + except os.error: + onerror(os.unlinkat, + os.path.join(path, name), + sys.exc_info()) + except: + onerror(os.fwalk, path, sys.exc_info()) + + try: + os.rmdir(path) + except os.error: + onerror(os.rmdir, path, sys.exc_info()) + +def _default_rmtree(path, ignore_errors=False, onerror=None): + """Recursively delete a directory tree. + + If ignore_errors is set, errors are ignored; otherwise, if onerror is set, + it is called to handle the error with arguments (func, path, exc_info) + where func is os.path.islink, os.listdir, os.remove, or os.rmdir; path is + the argument to that function that caused it to fail; and exc_info is + a tuple returned by sys.exc_info(). If ignore_errors is false and onerror + is None, an exception is raised. """ if ignore_errors: @@ -371,6 +423,10 @@ except os.error: onerror(os.rmdir, path, sys.exc_info()) +if hasattr(os, 'fwalk') and hasattr(os, 'unlinkat'): + rmtree = _safe_fwalk_rmtree +else: + rmtree = _default_rmtree def _basename(path): # A basename() variant which first strips the trailing slash, if present. diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py --- a/Lib/test/test_shutil.py +++ b/Lib/test/test_shutil.py @@ -120,29 +120,36 @@ def test_on_error(self): self.errorState = 0 os.mkdir(TESTFN) - self.childpath = os.path.join(TESTFN, 'a') - support.create_empty_file(self.childpath) + self.child_file_path = os.path.join(TESTFN, 'a') + self.child_dir_path = os.path.join(TESTFN, 'b') + support.create_empty_file(self.child_file_path) + os.mkdir(self.child_dir_path) old_dir_mode = os.stat(TESTFN).st_mode - old_child_mode = os.stat(self.childpath).st_mode + old_child_file_mode = os.stat(self.child_file_path).st_mode + old_child_dir_mode = os.stat(self.child_dir_path).st_mode # Make unwritable. - os.chmod(self.childpath, stat.S_IREAD) - os.chmod(TESTFN, stat.S_IREAD) + new_mode = stat.S_IREAD|stat.S_IEXEC + os.chmod(self.child_file_path, new_mode) + os.chmod(self.child_dir_path, new_mode) + os.chmod(TESTFN, new_mode) shutil.rmtree(TESTFN, onerror=self.check_args_to_onerror) # Test whether onerror has actually been called. - self.assertEqual(self.errorState, 2, - "Expected call to onerror function did not happen.") + self.assertEqual(self.errorState, 3, + "Expected call to onerror function did not " + "happen.") # Make writable again. os.chmod(TESTFN, old_dir_mode) - os.chmod(self.childpath, old_child_mode) + os.chmod(self.child_file_path, old_child_file_mode) + os.chmod(self.child_dir_path, old_child_dir_mode) # Clean up. shutil.rmtree(TESTFN) def check_args_to_onerror(self, func, arg, exc): # test_rmtree_errors deliberately runs rmtree - # on a directory that is chmod 400, which will fail. + # on a directory that is chmod 500, which will fail. # This function is run when shutil.rmtree fails. # 99.9% of the time it initially fails to remove # a file in the directory, so the first time through @@ -151,20 +158,45 @@ # FUSE experienced a failure earlier in the process # at os.listdir. The first failure may legally # be either. - if self.errorState == 0: - if func is os.remove: - self.assertEqual(arg, self.childpath) + if 0 <= self.errorState < 2: + if (func is os.remove or + hasattr(os, 'unlinkat') and func is os.unlinkat): + self.assertIn(arg, [self.child_file_path, self.child_dir_path]) else: - self.assertIs(func, os.listdir, - "func must be either os.remove or os.listdir") - self.assertEqual(arg, TESTFN) + if shutil._safe_fwalk_rmtree is shutil.rmtree: + self.assertIs(func, os.fwalk, + "func must be os.fwalk when rmtree is " + "_safe_fwalk_rmtree") + elif self.errorState == 1: + self.assertEqual(func, os.rmdir) + else: + self.assertIs(func, os.listdir, + "func must be os.listdir when rmtree is NOT " + "_safe_fwalk_rmtree") + self.assertIn(arg, [TESTFN, self.child_dir_path]) self.assertTrue(issubclass(exc[0], OSError)) - self.errorState = 1 + self.errorState += 1 else: self.assertEqual(func, os.rmdir) self.assertEqual(arg, TESTFN) self.assertTrue(issubclass(exc[0], OSError)) - self.errorState = 2 + self.errorState = 3 + + def test_rmtree_does_not_choke_on_failing_lstat(self): + try: + orig_lstat = os.lstat + def raiser(fn): + if fn != TESTFN: + raise OSError() + else: + return orig_lstat(fn) + os.lstat = raiser + + os.mkdir(TESTFN) + write_file((TESTFN, 'foo'), 'foo') + shutil.rmtree(TESTFN) + finally: + os.lstat = orig_lstat @unittest.skipUnless(hasattr(os, 'chmod'), 'requires os.chmod') @support.skip_unless_symlink @@ -629,6 +661,7 @@ os.mkdir(src) os.symlink(src, dst) self.assertRaises(OSError, shutil.rmtree, dst) + shutil.rmtree(dst, ignore_errors=True) finally: shutil.rmtree(TESTFN, ignore_errors=True)