From 563ccded6e83bfdd8c5622663c4edb679e96e08b Mon Sep 17 00:00:00 2001
From: Serhiy Storchaka <storchaka@gmail.com>
Date: Tue, 5 Dec 2023 17:40:49 +0200
Subject: [PATCH] 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.
---
 Lib/shutil.py                                 |  4 +--
 Lib/test/test_shutil.py                       | 33 +++++++++----------
 ...3-12-05-16-20-40.gh-issue-94692.-e5C3c.rst |  4 +++
 3 files changed, 22 insertions(+), 19 deletions(-)
 create mode 100644 Misc/NEWS.d/next/Library/2023-12-05-16-20-40.gh-issue-94692.-e5C3c.rst

diff --git a/Lib/shutil.py b/Lib/shutil.py
index 93b00d73a0fd46..bdbbcbc282e266 100644
--- a/Lib/shutil.py
+++ b/Lib/shutil.py
@@ -768,13 +768,13 @@ def onexc(*args):
         # lstat()/open()/fstat() trick.
         try:
             orig_st = os.lstat(path, dir_fd=dir_fd)
-        except Exception as err:
+        except OSError as err:
             onexc(os.lstat, path, err)
             return
         try:
             fd = os.open(path, os.O_RDONLY, dir_fd=dir_fd)
             fd_closed = False
-        except Exception as err:
+        except OSError as err:
             onexc(os.open, path, err)
             return
         try:
diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py
index 7ea2496230da47..9b8ec42a99dd69 100644
--- a/Lib/test/test_shutil.py
+++ b/Lib/test/test_shutil.py
@@ -317,7 +317,7 @@ def test_rmtree_works_on_junctions(self):
         self.assertTrue(os.path.exists(dir3))
         self.assertTrue(os.path.exists(file1))
 
-    def test_rmtree_errors_onerror(self):
+    def test_rmtree_errors(self):
         # filename is guaranteed not to exist
         filename = tempfile.mktemp(dir=self.mkdtemp())
         self.assertRaises(FileNotFoundError, shutil.rmtree, filename)
@@ -326,8 +326,8 @@ def test_rmtree_errors_onerror(self):
 
         # existing file
         tmpdir = self.mkdtemp()
-        write_file((tmpdir, "tstfile"), "")
         filename = os.path.join(tmpdir, "tstfile")
+        write_file(filename, "")
         with self.assertRaises(NotADirectoryError) as cm:
             shutil.rmtree(filename)
         self.assertEqual(cm.exception.filename, filename)
@@ -335,6 +335,19 @@ def test_rmtree_errors_onerror(self):
         # test that ignore_errors option is honored
         shutil.rmtree(filename, ignore_errors=True)
         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 = []
         def onerror(*args):
             errors.append(args)
@@ -350,23 +363,9 @@ def onerror(*args):
         self.assertEqual(errors[1][2][1].filename, filename)
 
     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()
-        write_file((tmpdir, "tstfile"), "")
         filename = os.path.join(tmpdir, "tstfile")
-        with self.assertRaises(NotADirectoryError) as cm:
-            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))
+        write_file(filename, "")
         errors = []
         def onexc(*args):
             errors.append(args)
diff --git a/Misc/NEWS.d/next/Library/2023-12-05-16-20-40.gh-issue-94692.-e5C3c.rst b/Misc/NEWS.d/next/Library/2023-12-05-16-20-40.gh-issue-94692.-e5C3c.rst
new file mode 100644
index 00000000000000..c67ba6c9ececdb
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2023-12-05-16-20-40.gh-issue-94692.-e5C3c.rst
@@ -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.