gh-94692: Only catch OSError in shutil.rmtree() (#112756)

Previously a symlink attack resistant version of shutil.rmtree() could ignore
or pass to the error handler arbitrary exception when invalid arguments
were provided.
This commit is contained in:
Serhiy Storchaka 2023-12-05 17:40:49 +02:00 committed by GitHub
parent bc68f4a4ab
commit 563ccded6e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 22 additions and 19 deletions

View File

@ -768,13 +768,13 @@ def rmtree(path, ignore_errors=False, onerror=None, *, onexc=None, dir_fd=None):
# lstat()/open()/fstat() trick. # lstat()/open()/fstat() trick.
try: try:
orig_st = os.lstat(path, dir_fd=dir_fd) orig_st = os.lstat(path, dir_fd=dir_fd)
except Exception as err: except OSError as err:
onexc(os.lstat, path, err) onexc(os.lstat, path, err)
return return
try: try:
fd = os.open(path, os.O_RDONLY, dir_fd=dir_fd) fd = os.open(path, os.O_RDONLY, dir_fd=dir_fd)
fd_closed = False fd_closed = False
except Exception as err: except OSError as err:
onexc(os.open, path, err) onexc(os.open, path, err)
return return
try: try:

View File

@ -317,7 +317,7 @@ class TestRmTree(BaseTest, unittest.TestCase):
self.assertTrue(os.path.exists(dir3)) self.assertTrue(os.path.exists(dir3))
self.assertTrue(os.path.exists(file1)) self.assertTrue(os.path.exists(file1))
def test_rmtree_errors_onerror(self): def test_rmtree_errors(self):
# filename is guaranteed not to exist # filename is guaranteed not to exist
filename = tempfile.mktemp(dir=self.mkdtemp()) filename = tempfile.mktemp(dir=self.mkdtemp())
self.assertRaises(FileNotFoundError, shutil.rmtree, filename) self.assertRaises(FileNotFoundError, shutil.rmtree, filename)
@ -326,8 +326,8 @@ class TestRmTree(BaseTest, unittest.TestCase):
# existing file # existing file
tmpdir = self.mkdtemp() tmpdir = self.mkdtemp()
write_file((tmpdir, "tstfile"), "")
filename = os.path.join(tmpdir, "tstfile") filename = os.path.join(tmpdir, "tstfile")
write_file(filename, "")
with self.assertRaises(NotADirectoryError) as cm: with self.assertRaises(NotADirectoryError) as cm:
shutil.rmtree(filename) shutil.rmtree(filename)
self.assertEqual(cm.exception.filename, filename) self.assertEqual(cm.exception.filename, filename)
@ -335,6 +335,19 @@ class TestRmTree(BaseTest, unittest.TestCase):
# test that ignore_errors option is honored # test that ignore_errors option is honored
shutil.rmtree(filename, ignore_errors=True) shutil.rmtree(filename, ignore_errors=True)
self.assertTrue(os.path.exists(filename)) self.assertTrue(os.path.exists(filename))
self.assertRaises(TypeError, shutil.rmtree, None)
self.assertRaises(TypeError, shutil.rmtree, None, ignore_errors=True)
exc = TypeError if shutil.rmtree.avoids_symlink_attacks else NotImplementedError
with self.assertRaises(exc):
shutil.rmtree(filename, dir_fd='invalid')
with self.assertRaises(exc):
shutil.rmtree(filename, dir_fd='invalid', ignore_errors=True)
def test_rmtree_errors_onerror(self):
tmpdir = self.mkdtemp()
filename = os.path.join(tmpdir, "tstfile")
write_file(filename, "")
errors = [] errors = []
def onerror(*args): def onerror(*args):
errors.append(args) errors.append(args)
@ -350,23 +363,9 @@ class TestRmTree(BaseTest, unittest.TestCase):
self.assertEqual(errors[1][2][1].filename, filename) self.assertEqual(errors[1][2][1].filename, filename)
def test_rmtree_errors_onexc(self): def test_rmtree_errors_onexc(self):
# filename is guaranteed not to exist
filename = tempfile.mktemp(dir=self.mkdtemp())
self.assertRaises(FileNotFoundError, shutil.rmtree, filename)
# test that ignore_errors option is honored
shutil.rmtree(filename, ignore_errors=True)
# existing file
tmpdir = self.mkdtemp() tmpdir = self.mkdtemp()
write_file((tmpdir, "tstfile"), "")
filename = os.path.join(tmpdir, "tstfile") filename = os.path.join(tmpdir, "tstfile")
with self.assertRaises(NotADirectoryError) as cm: write_file(filename, "")
shutil.rmtree(filename)
self.assertEqual(cm.exception.filename, filename)
self.assertTrue(os.path.exists(filename))
# test that ignore_errors option is honored
shutil.rmtree(filename, ignore_errors=True)
self.assertTrue(os.path.exists(filename))
errors = [] errors = []
def onexc(*args): def onexc(*args):
errors.append(args) errors.append(args)

View File

@ -0,0 +1,4 @@
:func:`shutil.rmtree` now only catches OSError exceptions. Previously a
symlink attack resistant version of ``shutil.rmtree()`` could ignore or pass
to the error handler arbitrary exception when invalid arguments were
provided.