Skip to content

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

Open
serhiy-storchaka wants to merge 6 commits intopython:mainfrom
serhiy-storchaka:shutil-unpack_archive-extractall
Open

gh-146581: Fix vulnerability in shutil.unpack_archive() for ZIP files on Windows#146591
serhiy-storchaka wants to merge 6 commits intopython:mainfrom
serhiy-storchaka:shutil-unpack_archive-extractall

Conversation

@serhiy-storchaka
Copy link
Copy Markdown
Member

@serhiy-storchaka serhiy-storchaka commented Mar 29, 2026

Use ZipFile.extractall() to sanitize file names and extract files.

Files with invalid names (e.g. absolute paths) are now skipped.

Files containing ".." in the name are no longer skipped.

Use ZipFile.extractall() to sanitize file names and extract files.

Files with invalid names (e.g. absolute paths) are now extracted with
different names instead of been skipped or written out of the destination
directory.

Files containing ".." in the name are no longer skipped.
…ve-extractall' into shutil-unpack_archive-extractall
@Shrey-N
Copy link
Copy Markdown
Contributor

Shrey-N commented Mar 29, 2026

Wasn't os.path.splitroot() introduced in 3.12? if it is backported wouldn't it crash with AttributeError?

@Shrey-N
Copy link
Copy Markdown
Contributor

Shrey-N commented Mar 29, 2026

Also, the new os.path.pardir check misses backslash based traversals on Linux/macOS.
The old substring check if '..' in name caught these, but the new component based check treats that whole string as a single (but malicious) filename.

@serhiy-storchaka
Copy link
Copy Markdown
Member Author

Wasn't os.path.splitroot() introduced in 3.12? if it is backported wouldn't it crash with AttributeError?

Backports will be fixed to use equivalent code.

Also, the new os.path.pardir check misses backslash based traversals on Linux/macOS.

Backslash is not a separator on Posix. It is a legal character which has no special meaning.

@Shrey-N
Copy link
Copy Markdown
Contributor

Shrey-N commented Mar 29, 2026

I believe it’s actually a regression. Currently, shutil uses a substring check which correctly catches and skips these backslash traversals on Linux.

In this PR it switches to a component based check, which removes that existing protection.

So I believe this actually reduces the existing security strictness for non windows users

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting core review needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes type-security A security issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants