Skip to content

Resolve symlinks in the zip extraction containment check - #14327

Open
dkautomation23 wants to merge 3 commits into
pypa:mainfrom
dkautomation23:fix/zip-containment-resolve-symlinks
Open

dkautomation23 wants to merge 3 commits into
pypa:mainfrom
dkautomation23:fix/zip-containment-resolve-symlinks

Conversation

@dkautomation23

Copy link
Copy Markdown

What does this PR do?

The tar fallback path checks containment twice: once on the textual path and once with symlinks resolved. The zip path only did the textual check. Apply the same resolved check in unzip_file so both archive formats enforce the same invariant, and add a test for a member written through a symlink that already exists in the destination.

Unit suite on Linux / Python 3.13: 1,929 passed, 0 failed.

PR Checklist:

  • I agree to follow the PSF Code of Conduct.
  • I have read and have followed the CONTRIBUTING.md file.
  • I have added a news file fragment (or this PR does not need one).
  • I have read and followed the AI_POLICY.md file, and if any AI tools were used, I have disclosed it below.

Assisted-by: Claude

The tar fallback path checks containment twice: once on the textual path
and once with symlinks resolved. The zip path only did the textual check.
Apply the same resolved check in unzip_file so both archive formats enforce
the same invariant, and add a test for a member written through a symlink
that already exists in the destination.

@2ykwang 2ykwang left a comment

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.

Could you share an example of how this can happen? unzip_file() doesn't seem to create symlinks.

This branch has not been deployed

No deployments
Sign up for free to 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.

2 participants