FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-146581: Fix vulnerability in shutil.unpack_archive() for ZIP files on Windows by serhiy-storchaka · Pull Request #146591 · python/cpython · GitHub

/ cpython Public

gh-146581: Fix vulnerability in shutil.unpack_archive() for ZIP files on Windows #146591

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
serhiy-storchaka merged 6 commits into from
Apr 27, 2026
+89 −28
Conversation 14 Commits 6 Checks 77 Files changed 4
Merged

gh-146581: Fix vulnerability in shutil.unpack_archive() for ZIP files on Windows #146591

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7c5ad11
Fix vulnerability in shutil.unpack_archive() in ZIP files
serhiy-storchaka Feb 27, 2026
8d0ade5
Preserve the current behavior whether it is possible.
serhiy-storchaka Feb 27, 2026
7940084
Preserve the current behavior whether it is possible.
serhiy-storchaka Feb 27, 2026
faddc67
Merge remote-tracking branch 'refs/remotes/origin/shutil-unpack_archi…
serhiy-storchaka Mar 29, 2026
76797cc
Merge branch 'main' into shutil-unpack_archive-extractall
serhiy-storchaka Mar 29, 2026
12cf735
Add a NEWS entry.
serhiy-storchaka Mar 29, 2026
File filter

Filter by extension

Filter by extension .py  (3) .rst  (1) All 2 file types selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
24 changes: 3 additions & 21 deletions Lib/shutil.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -1317,27 +1317,9 @@ def _unpack_zipfile(filename, extract_dir):
if not zipfile.is_zipfile(filename):
raise ReadError("%s is not a zip file" % filename)

zip = zipfile.ZipFile(filename)
try:
for info in zip.infolist():
name = info.filename

# don't extract absolute paths or ones with .. in them
if name.startswith('/') or '..' in name:
continue

targetpath = os.path.join(extract_dir, *name.split('/'))
if not targetpath:
continue

_ensure_directory(targetpath)
if not name.endswith('/'):
# file
with zip.open(name, 'r') as source, \
open(targetpath, 'wb') as target:
copyfileobj(source, target)
finally:
zip.close()
with zipfile.ZipFile(filename) as zip:
zip._ignore_invalid_names = True
zip.extractall(extract_dir)

def _unpack_tarfile(filename, extract_dir, *, filter=None):
"""Unpack tar/tar.gz/tar.bz2/tar.xz/tar.zst `filename` to `extract_dir`
Expand Down
67 changes: 65 additions & 2 deletions Lib/test/test_shutil.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -2136,8 +2136,6 @@ def test_make_zipfile_rootdir_nodir(self):
def check_unpack_archive(self, format, **kwargs):
self.check_unpack_archive_with_converter(
format, lambda path: path, **kwargs)
self.check_unpack_archive_with_converter(
format, FakePath, **kwargs)
self.check_unpack_archive_with_converter(format, FakePath, **kwargs)

def check_unpack_archive_with_converter(self, format, converter, **kwargs):
Expand Down Expand Up @@ -2194,6 +2192,71 @@ def test_unpack_archive_zip(self):
with self.assertRaises(TypeError):
self.check_unpack_archive('zip', filter='data')

def test_unpack_archive_zip_badpaths(self):
srcdir = self.mkdtemp()
zipname = os.path.join(srcdir, 'test.zip')
abspath = os.path.join(srcdir, 'abspath')
with zipfile.ZipFile(zipname, 'w') as zf:
zf.writestr(abspath, 'badfile')
zf.writestr(os.sep + abspath, 'badfile')
zf.writestr('/abspath', 'badfile')
zf.writestr('C:/abspath', 'badfile')
zf.writestr('D:\\abspath', 'badfile')
zf.writestr('E:abspath', 'badfile')
zf.writestr('F:/G:/abspath', 'badfile')
zf.writestr('//server/share/abspath', 'badfile')
zf.writestr('\\\\server2\\share\\abspath', 'badfile')
zf.writestr('../relpath', 'badfile')
zf.writestr(os.pardir + os.sep + 'relpath2', 'badfile')
zf.writestr('good/file', 'goodfile')
zf.writestr('good..file', 'goodfile')

dstdir = os.path.join(self.mkdtemp(), 'dst')
unpack_archive(zipname, dstdir)
self.assertTrue(os.path.isfile(os.path.join(dstdir, 'good', 'file')))
self.assertTrue(os.path.isfile(os.path.join(dstdir, 'good..file')))
self.assertFalse(os.path.exists(abspath))
self.assertFalse(os.path.exists(os.path.join(dstdir, 'abspath')))
self.assertFalse(os.path.exists(os.path.join(dstdir, 'G_')))
self.assertFalse(os.path.exists(os.path.join(dstdir, 'server')))
if os.name != 'nt':
self.assertTrue(os.path.isfile(os.path.join(dstdir, 'C:', 'abspath')))
self.assertTrue(os.path.isfile(os.path.join(dstdir, 'D:\\abspath')))
self.assertTrue(os.path.isfile(os.path.join(dstdir, 'E:abspath')))
self.assertTrue(os.path.isfile(os.path.join(dstdir, 'F:', 'G:', 'abspath')))
self.assertTrue(os.path.isfile(os.path.join(dstdir, '\\\\server2\\share\\abspath')))
if os.pardir == '..':
self.assertFalse(os.path.exists(os.path.join(dstdir, '..', 'relpath')))
self.assertFalse(os.path.exists(os.path.join(dstdir, 'relpath')))
else:
self.assertTrue(os.path.isfile(os.path.join(dstdir, '..', 'relpath')))
self.assertFalse(os.path.exists(os.path.join(dstdir, os.pardir, 'relpath2')))
self.assertFalse(os.path.exists(os.path.join(dstdir, 'relpath2')))

dstdir2 = os.path.join(self.mkdtemp(), 'dst')
os.mkdir(dstdir2)
with os_helper.change_cwd(dstdir2):
unpack_archive(zipname, '')
self.assertTrue(os.path.isfile(os.path.join('good', 'file')))
self.assertTrue(os.path.isfile('good..file'))
self.assertFalse(os.path.exists(abspath))
self.assertFalse(os.path.exists('abspath'))
self.assertFalse(os.path.exists('C_'))
self.assertFalse(os.path.exists('server'))
if os.name != 'nt':
self.assertTrue(os.path.isfile(os.path.join('C:', 'abspath')))
self.assertTrue(os.path.isfile('D:\\abspath'))
self.assertTrue(os.path.isfile('E:abspath'))
self.assertTrue(os.path.isfile(os.path.join('F:', 'G:', 'abspath')))
self.assertTrue(os.path.isfile('\\\\server2\\share\\abspath'))
if os.pardir == '..':
self.assertFalse(os.path.exists(os.path.join('..', 'relpath')))
self.assertFalse(os.path.exists('relpath'))
else:
self.assertTrue(os.path.isfile(os.path.join('..', 'relpath')))
self.assertFalse(os.path.exists(os.path.join(os.pardir, 'relpath2')))
self.assertFalse(os.path.exists('relpath2'))

def test_unpack_registry(self):

formats = get_unpack_formats()
Expand Down
21 changes: 16 additions & 5 deletions Lib/zipfile/__init__.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -1410,6 +1410,7 @@ class ZipFile:

fp = None # Set here since __del__ checks it
_windows_illegal_name_trans_table = None
_ignore_invalid_names = False

zooba Apr 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Is this intended to be set by users needing back-compat as well? Should we document it (carefully)?

serhiy-storchaka Apr 15, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

It is intended for internal use in shutil.unpack_archive() only.

We could add it as optional parameter, but this does not look like a good long term solution. Even if we make the parameter name underscored, it will be visible in the help, and will confuse users. Users will start to use it, so it will be difficult to get rid of it.

I think that in future versions we can add a hook which would be called for all entry names. It would allow to translate them to valid paths, allow to skip them (with possible logging) or raise an exception. But this is a new feature, it cannot be backported. And it needs a separate discussion.

def __init__(self, file, mode="r", compression=ZIP_STORED, allowZip64=True,
compresslevel=None, *, strict_timestamps=True, metadata_encoding=None):
Expand Down Expand Up @@ -1890,21 +1891,31 @@ def _extract_member(self, member, targetpath, pwd):

# build the destination pathname, replacing
# forward slashes to platform specific separators.
arcname = member.filename.replace('/', os.path.sep)

if os.path.altsep:
arcname = member.filename
if os.path.sep != '/':
arcname = arcname.replace('/', os.path.sep)
if os.path.altsep and os.path.altsep != '/':
arcname = arcname.replace(os.path.altsep, os.path.sep)
# interpret absolute pathname as relative, remove drive letter or
# UNC path, redundant separators, "." and ".." components.
arcname = os.path.splitdrive(arcname)[1]
drive, root, arcname = os.path.splitroot(arcname)
if self._ignore_invalid_names and (drive or root):
return None
if self._ignore_invalid_names and os.path.pardir in arcname.split(os.path.sep):
return None
invalid_path_parts = ('', os.path.curdir, os.path.pardir)
arcname = os.path.sep.join(x for x in arcname.split(os.path.sep)
if x not in invalid_path_parts)
if os.path.sep == '\\':
# filter illegal characters on Windows
arcname = self._sanitize_windows_name(arcname, os.path.sep)
arcname2 = self._sanitize_windows_name(arcname, os.path.sep)
if self._ignore_invalid_names and arcname2 != arcname:
return None
arcname = arcname2

if not arcname and not member.is_dir():
if self._ignore_invalid_names:
return None
raise ValueError("Empty filename.")

targetpath = os.path.join(targetpath, arcname)
Expand Down
5 changes: 5 additions & 0 deletions Misc/NEWS.d/next/Security/2026-03-29-12-51-33.gh-issue-146581.4vZfB0.rst
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Fix vulnerability in :func:`shutil.unpack_archive` for ZIP files on Windows
which allowed to write files outside of the destination tree if the patch in
the archive contains a Windows drive prefix. Now such invalid paths will be
skipped. Files containing ".." in the name (like "foo..bar") are no longer
skipped.
Loading
Back | FazBrowse Home | New Git URL