GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-127987: Ensure that directories are not renamed during `tar.TarFile.extractall()` by noamcohen97 · Pull Request #134628 · python/cpython

Conversation

@noamcohen97

@noamcohen97 noamcohen97 commented May 24, 2025 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

The most straightforward approach to solving this issue is to re-consult the filter_function just before updating the attributes in the directory within extract_all(). However, this would result in stateful extraction filters receiving a redundant call for a file that has already passed the filter.

Inspired by GNU Tar, I decided to save the exact inode number of the created directory to ensure it hasn't changed.
The only downside I can see is that stat() is called shortly after the directory is created, which introduces a race condition. Since the entire module is already susceptible to these kinds of races (the path is resolved in the filter_function and then re-resolved in the actual file creation methods), I believe this is acceptable.

  • Issue: TarFile.extractall(..., filter='tar') arbitrary file chmod #127987

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
noamcohen97 added 3 commits May 24, 2025 14:48
@noamcohen97
add a test case to replicate the issue
d95991d
@noamcohen97
extract _transform_destination_path()
2463a85
@noamcohen97
make sure the directory is not renamed during extractall()
d97e83e
@bedevere-app bedevere-app Bot mentioned this pull request May 24, 2025
TarFile.extractall(..., filter='tar') arbitrary file chmod #127987
Closed
@noamcohen97

noamcohen97 commented May 24, 2025

Copy link
Copy Markdown
Contributor Author

@sethmlarson Please let me know what your thoughts are on this one

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

noamcohen97 added 4 commits May 24, 2025 16:38
@noamcohen97
do not follow symlinks
a4a481c
@noamcohen97
remove unneeded mocks
2140b19
@noamcohen97
do not run on systems without os.chmod()
6b20f0d
@noamcohen97
simplify test
491b40f
@noamcohen97
noamcohen97 force-pushed the tar-arbitrary-file-chmod branch from 31dddea to ba8c0f5 Compare May 24, 2025 14:52
@noamcohen97
move test to MiscTest
0e2d157
@noamcohen97
noamcohen97 force-pushed the tar-arbitrary-file-chmod branch from ba8c0f5 to 0e2d157 Compare May 24, 2025 16:07
@noamcohen97
remove part of poc that used to bypass data filter
7bd7b85
it doesn't have to do with this bug
sharktide
sharktide suggested changes May 24, 2025
View reviewed changes

@sharktide sharktide left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

A news entry would be best here

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@bedevere-app bedevere-app Bot added the awaiting core review label May 24, 2025
@noamcohen97
inode re-use guard
c7e4e05
@github-actions

github-actions Bot commented Apr 25, 2026

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 25, 2026
@StanFromIreland StanFromIreland closed this Aug 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ethanfurman ethanfurman Awaiting requested review from ethanfurman ethanfurman will be requested when the pull request is marked ready for review ethanfurman is a code owner
1 more reviewer
@sharktide sharktide sharktide requested changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting core review stale Stale PR or inactive for long period of time.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@noamcohen97 @sharktide @StanFromIreland
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.