Skip to content

gh-146581: Use ZipFile.extractall() in shutil for secure ZIP extraction - #146588

Closed
Shrey-N wants to merge 8 commits into
python:mainfrom
Shrey-N:patch-2
Closed

gh-146581: Use ZipFile.extractall() in shutil for secure ZIP extraction#146588
Shrey-N wants to merge 8 commits into
python:mainfrom
Shrey-N:patch-2

Conversation

@Shrey-N

@Shrey-NShrey-N commented Mar 29, 2026

Copy link
Copy Markdown
Contributor

This PR refactors shutil._unpack_zipfile to use zipfile.ZipFile.extractall() instead of a manual extraction loop. This resolves a directory traversal vulnerability on Windows where archives containing drive prefixed paths for exampleD:/file.txt could write files outside the intended destination directory.

Changes Made

  • Replaced manual path joining and validation with ZipFile.extractall(), which leverages the zipfile module
  • Updated the _unpack_zipfile signature to accept **kwargs. This ensures compatibility with the filter argument, preventing TypeError while allowing future filter support for ZIP files.
  • This change brings ZIP extraction in line with how shutil handles TAR files, which already uses extractall().

Regression Test

Added a new test case test_unpack_zipfile_traversal_windows_drive to Lib/test/test_shutil.py. This test specifically verifies that a ZIP file containing a drive prefixed path is safely sanitized and extracted within the intended extract_dir on Windows.

Linked Issue

#146581

Add a test case for ZIP file traversal on Windows.
Updated comments for clarity and removed redundant lines.
@StanFromIreland

Copy link
Copy Markdown
Member

Closing in favour of #146591

@serhiy-storchaka

Copy link
Copy Markdown
Member

Thank you for your PR, @Shrey-N, but we already discussed this issue and created a solution. #146591 is in general similar to your PR, but includes results of that discussion.

@Shrey-N

Copy link
Copy Markdown
ContributorAuthor

Thank you for the update @serhiy-storchaka. I am glad to see the fix landing via #146591 :)

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Shrey-N@StanFromIreland@serhiy-storchaka