NotificationsYou must be signed in to change notification settings
Fork33.3k
Star69.8k

Commit563ccde

authored

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

Previously a symlink attack resistant version of shutil.rmtree() could ignoreor pass to the error handler arbitrary exception when invalid argumentswere provided.

1 parentbc68f4a commit563ccdeCopy full SHA for 563ccde

File tree

3 files changed

+22

-19

lines changed

Lib
- shutil.py
- test
  - test_shutil.py
Misc/NEWS.d/next/Library
- 2023-12-05-16-20-40.gh-issue-94692.-e5C3c.rst

3 files changed

+22

-19

lines changed

`‎Lib/shutil.py‎`

Lines changed: 2 additions & 2 deletions

Original file line number	Diff line number	Diff line change
`@@ -768,13 +768,13 @@ def onexc(*args):`
`768`	`768`	`# lstat()/open()/fstat() trick.`
`769`	`769`	`try:`
`770`	`770`	`orig_st=os.lstat(path,dir_fd=dir_fd)`
`771`		`-exceptExceptionaserr:`
	`771`	`+exceptOSErroraserr:`
`772`	`772`	`onexc(os.lstat,path,err)`
`773`	`773`	`return`
`774`	`774`	`try:`
`775`	`775`	`fd=os.open(path,os.O_RDONLY,dir_fd=dir_fd)`
`776`	`776`	`fd_closed=False`
`777`		`-exceptExceptionaserr:`
	`777`	`+exceptOSErroraserr:`
`778`	`778`	`onexc(os.open,path,err)`
`779`	`779`	`return`
`780`	`780`	`try:`

`‎Lib/test/test_shutil.py‎`

Lines changed: 16 additions & 17 deletions

Original file line number	Diff line number	Diff line change
`@@ -317,7 +317,7 @@ def test_rmtree_works_on_junctions(self):`
`317`	`317`	`self.assertTrue(os.path.exists(dir3))`
`318`	`318`	`self.assertTrue(os.path.exists(file1))`
`319`	`319`
`320`		`-deftest_rmtree_errors_onerror(self):`
	`320`	`+deftest_rmtree_errors(self):`
`321`	`321`	`# filename is guaranteed not to exist`
`322`	`322`	`filename=tempfile.mktemp(dir=self.mkdtemp())`
`323`	`323`	`self.assertRaises(FileNotFoundError,shutil.rmtree,filename)`
`@@ -326,15 +326,28 @@ def test_rmtree_errors_onerror(self):`
`326`	`326`
`327`	`327`	`# existing file`
`328`	`328`	`tmpdir=self.mkdtemp()`
`329`		`-write_file((tmpdir,"tstfile"),"")`
`330`	`329`	`filename=os.path.join(tmpdir,"tstfile")`
	`330`	`+write_file(filename,"")`
`331`	`331`	`withself.assertRaises(NotADirectoryError)ascm:`
`332`	`332`	`shutil.rmtree(filename)`
`333`	`333`	`self.assertEqual(cm.exception.filename,filename)`
`334`	`334`	`self.assertTrue(os.path.exists(filename))`
`335`	`335`	`# test that ignore_errors option is honored`
`336`	`336`	`shutil.rmtree(filename,ignore_errors=True)`
`337`	`337`	`self.assertTrue(os.path.exists(filename))`
	`338`	`+`
	`339`	`+self.assertRaises(TypeError,shutil.rmtree,None)`
	`340`	`+self.assertRaises(TypeError,shutil.rmtree,None,ignore_errors=True)`
	`341`	`+exc=TypeErrorifshutil.rmtree.avoids_symlink_attackselseNotImplementedError`
	`342`	`+withself.assertRaises(exc):`
	`343`	`+shutil.rmtree(filename,dir_fd='invalid')`
	`344`	`+withself.assertRaises(exc):`
	`345`	`+shutil.rmtree(filename,dir_fd='invalid',ignore_errors=True)`
	`346`	`+`
	`347`	`+deftest_rmtree_errors_onerror(self):`
	`348`	`+tmpdir=self.mkdtemp()`
	`349`	`+filename=os.path.join(tmpdir,"tstfile")`
	`350`	`+write_file(filename,"")`
`338`	`351`	`errors= []`
`339`	`352`	`defonerror(*args):`
`340`	`353`	`errors.append(args)`
`@@ -350,23 +363,9 @@ def onerror(*args):`
`350`	`363`	`self.assertEqual(errors[1][2][1].filename,filename)`
`351`	`364`
`352`	`365`	`deftest_rmtree_errors_onexc(self):`
`353`		`-# filename is guaranteed not to exist`
`354`		`-filename=tempfile.mktemp(dir=self.mkdtemp())`
`355`		`-self.assertRaises(FileNotFoundError,shutil.rmtree,filename)`
`356`		`-# test that ignore_errors option is honored`
`357`		`-shutil.rmtree(filename,ignore_errors=True)`
`358`		`-`
`359`		`-# existing file`
`360`	`366`	`tmpdir=self.mkdtemp()`
`361`		`-write_file((tmpdir,"tstfile"),"")`
`362`	`367`	`filename=os.path.join(tmpdir,"tstfile")`
`363`		`-withself.assertRaises(NotADirectoryError)ascm:`
`364`		`-shutil.rmtree(filename)`
`365`		`-self.assertEqual(cm.exception.filename,filename)`
`366`		`-self.assertTrue(os.path.exists(filename))`
`367`		`-# test that ignore_errors option is honored`
`368`		`-shutil.rmtree(filename,ignore_errors=True)`
`369`		`-self.assertTrue(os.path.exists(filename))`
	`368`	`+write_file(filename,"")`
`370`	`369`	`errors= []`
`371`	`370`	`defonexc(*args):`
`372`	`371`	`errors.append(args)`

`‎Misc/NEWS.d/next/Library/2023-12-05-16-20-40.gh-issue-94692.-e5C3c.rst‎`

Lines changed: 4 additions & 0 deletions

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

0 commit comments

Comments

(0)

Movatterモバイル変換

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Commit563ccde

File tree

3 files changed

3 files changed

`‎Lib/shutil.py‎`

`‎Lib/test/test_shutil.py‎`

`‎Misc/NEWS.d/next/Library/2023-12-05-16-20-40.gh-issue-94692.-e5C3c.rst‎`

0 commit comments