bpo-41316: Make tarfile follow specs for FNAME#21511

ArtemSBulgakov · 2020-07-16T18:42:35Z

tarfile writes full path to FNAME field of GZIP format instead of just basename if user specified absolute path. Some archive viewers may process file incorrectly. Also it creates security issue because anyone can know structure of directories on system and know username or other personal information.

RFC1952 says about FNAME:
This is the original name of the file being compressed, with any directory components removed.

So tarfile must remove directory names from FNAME and write only basename of file.

https://bugs.python.org/issue41316

Automerge-Triggered-By: @jaraco

ofek · 2020-07-16T19:04:13Z

Great find!

Lib/tarfile.py

ArtemSBulgakov · 2020-07-22T07:49:21Z

@ethanfurman Could you please review changes?

tarfile writes full path to FNAME field of GZIP format instead of just basename if user specified absolute path. Some archive viewers may process file incorrectly. Also it creates security issue because anyone can know structure of directories on system and know username or other personal information. RFC1952 says about FNAME: This is the original name of the file being compressed, with any directory components removed. So tarfile must remove directory names from FNAME and write only basename of file.

ArtemSBulgakov · 2020-08-18T12:12:25Z

Anyone?

Lib/tarfile.py

jaraco

Can you add a test to capture the expected behavior?

bedevere-bot · 2020-09-07T15:14:04Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

…sultant tarfile.

jaraco · 2020-09-07T16:24:27Z

In 3948880 , I added a failing test and in 0bbaa1c, merged it with the change, demonstrating the fix.

miss-islington · 2020-09-07T16:44:42Z

@ArtemSBulgakov: Status check is done, and it's a failure ❌ .

miss-islington · 2020-09-07T16:46:32Z

@ArtemSBulgakov: Status check is done, and it's a success ✅ .

miss-islington · 2020-09-07T16:46:36Z

Thanks @ArtemSBulgakov for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8, 3.9.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

@jaraco

tarfile writes full path to FNAME field of GZIP format instead of just basename if user specified absolute path. Some archive viewers may process file incorrectly. Also it creates security issue because anyone can know structure of directories on system and know username or other personal information. RFC1952 says about FNAME: This is the original name of the file being compressed, with any directory components removed. So tarfile must remove directory names from FNAME and write only basename of file. Automerge-Triggered-By: @jaraco (cherry picked from commit 22748a8) Co-authored-by: Artem Bulgakov <ArtemSBulgakov@ya.ru>

bedevere-bot · 2020-09-07T16:46:49Z

GH-22140 is a backport of this pull request to the 3.9 branch.

bedevere-bot · 2020-09-07T16:46:58Z

GH-22141 is a backport of this pull request to the 3.8 branch.

@jaraco

tarfile writes full path to FNAME field of GZIP format instead of just basename if user specified absolute path. Some archive viewers may process file incorrectly. Also it creates security issue because anyone can know structure of directories on system and know username or other personal information. RFC1952 says about FNAME: This is the original name of the file being compressed, with any directory components removed. So tarfile must remove directory names from FNAME and write only basename of file. Automerge-Triggered-By: @jaraco (cherry picked from commit 22748a8) Co-authored-by: Artem Bulgakov <ArtemSBulgakov@ya.ru>

* origin/master: (1373 commits) bpo-1635741: Port mashal module to multi-phase init (python#22149) bpo-1635741: Port _string module to multi-phase init (pythonGH-22148) bpo-1635741: Convert _sha256 types to heap types (pythonGH-22134) bpo-1635741: Port the termios to multi-phase init (PEP 489) (pythonGH-22139) bpo-41732: add iterator to memoryview (pythonGH-22119) bpo-40744: Drop support for SQLite pre 3.7.3 (pythonGH-20909) bpo-41316: Make tarfile follow specs for FNAME (pythonGH-21511) bpo-41720: Add "return NotImplemented" in turtle.Vec2D.__rmul__(). (pythonGH-22092) bpo-1635741 port _curses_panel to multi-phase init (PEP 489) (pythonGH-21986) bpo-1635741: Port _overlapped module to multi-phase init (pythonGH-22051) bpo-1635741: Port _opcode module to multi-phase init (PEP 489) (pythonGH-22050) bpo-1635741 port zlib module to multi-phase init (pythonGH-21995) [doc] Add link to Generic in typing (pythonGH-22125) bpo-41513: Expand comments and add references for a better understanding (pythonGH-22123) bpo-1635741: Port _sha1, _sha512, _md5 to multiphase init (pythonGH-21818) closes bpo-41723: Fix an error in the py_compile documentation. (pythonGH-22110) [doc] Fix padding in some typing definitions (pythonGH-22114) Fix documented Python version for venv --upgrade-deps (pythonGH-22113) bpo-40318: Migrate to SQLite3 trace v2 API (pythonGH-19581) bpo-41687: Fix sendfile implementation to work with Solaris (python#22040) ...

@jaraco

tarfile writes full path to FNAME field of GZIP format instead of just basename if user specified absolute path. Some archive viewers may process file incorrectly. Also it creates security issue because anyone can know structure of directories on system and know username or other personal information. RFC1952 says about FNAME: This is the original name of the file being compressed, with any directory components removed. So tarfile must remove directory names from FNAME and write only basename of file. Automerge-Triggered-By: @jaraco

@jaraco

tarfile writes full path to FNAME field of GZIP format instead of just basename if user specified absolute path. Some archive viewers may process file incorrectly. Also it creates security issue because anyone can know structure of directories on system and know username or other personal information. RFC1952 says about FNAME: This is the original name of the file being compressed, with any directory components removed. So tarfile must remove directory names from FNAME and write only basename of file. Automerge-Triggered-By: @jaraco (cherry picked from commit 22748a8) Co-authored-by: Artem Bulgakov <ArtemSBulgakov@ya.ru>

@jaraco

tarfile writes full path to FNAME field of GZIP format instead of just basename if user specified absolute path. Some archive viewers may process file incorrectly. Also it creates security issue because anyone can know structure of directories on system and know username or other personal information. RFC1952 says about FNAME: This is the original name of the file being compressed, with any directory components removed. So tarfile must remove directory names from FNAME and write only basename of file. Automerge-Triggered-By: @jaraco (cherry picked from commit 22748a8) Co-authored-by: Artem Bulgakov <ArtemSBulgakov@ya.ru>

kodonnell · 2021-12-06T16:58:03Z

I hit this in 3.7 - any reason it wasn't backported? Without knowing much about @miss-islington it looks like this just needs a needs backport to 3.7 label as @serhiy-storchaka did for 3.8/3.9. If there's anything else, happy to help.

miss-islington · 2021-12-06T17:09:19Z

Thanks @ArtemSBulgakov for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

miss-islington · 2021-12-06T17:09:21Z

Sorry @ArtemSBulgakov, I had trouble checking out the 3.7 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 22748a83d927d3da1beaed771be30887c42b2500 3.7

ethanfurman · 2021-12-06T17:10:57Z

Python 3.7 is now in security-maintenance mode, so no new binary releases. You'll have to build the updated Python yourself.

I'll try to get to the cherrypick this week if nobody else beats me to it.

ArtemSBulgakov requested a review from ethanfurman as a code owner July 16, 2020 18:42

the-knights-who-say-ni added the CLA signed label Jul 16, 2020

bedevere-bot added the awaiting review label Jul 16, 2020

ArtemSBulgakov mentioned this pull request Jul 16, 2020
sdist with --dist-dir on Windows produces incorrect archive pypa/setuptools#1185
Closed

eamanu reviewed Jul 17, 2020
View reviewed changes

Lib/tarfile.py Outdated Show resolvedHide resolved

ArtemSBulgakovand others added 3 commits July 28, 2020 17:00

📜🤖 Added by blurb_it.
6f751fe

Update ACKS
4a8bd29

jaraco reviewed Sep 7, 2020
View reviewed changes

Lib/tarfile.py Outdated Show resolvedHide resolved

jaraco requested changes Sep 7, 2020
View reviewed changes

bedevere-bot removed the awaiting review label Sep 7, 2020

bedevere-bot added the awaiting changes label Sep 7, 2020

jaraco added 3 commits September 7, 2020 12:22

Add test capturing missed expectation when the path appears in the re…
3948880
…sultant tarfile.

Merge branch 'bpo-41316-test' intobpo-41316
0bbaa1c

Reference RFC and precise language for easier searching.
4baaf5e

jaraco approved these changes Sep 7, 2020
View reviewed changes

bedevere-bot added awaiting merge and removed awaiting changes labels Sep 7, 2020

jaraco added the 🤖 automerge label Sep 7, 2020

serhiy-storchaka added needs backport to 3.8 type-bug An unexpected behavior, bug, or error labels Sep 7, 2020

miss-islington merged commit 22748a8 into python:masterSep 7, 2020

bedevere-bot removed the awaiting merge label Sep 7, 2020

bedevere-bot removed the needs backport to 3.9 label Sep 7, 2020

bedevere-bot removed the needs backport to 3.8 label Sep 7, 2020

ethanfurman added needs backport to 3.7 type-security A security issue labels Dec 6, 2021

miss-islington self-assigned this Dec 6, 2021

Uh oh!

bpo-41316: Make tarfile follow specs for FNAME#21511

bpo-41316: Make tarfile follow specs for FNAME #21511

Uh oh!

Conversation

ArtemSBulgakov commented Jul 16, 2020• edited by miss-islington Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ofek commented Jul 16, 2020

Uh oh!

Uh oh!

ArtemSBulgakov commented Jul 22, 2020

Uh oh!

ArtemSBulgakov commented Aug 18, 2020

Uh oh!

Uh oh!

jaraco left a comment

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Sep 7, 2020

Uh oh!

jaraco commented Sep 7, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

miss-islington commented Sep 7, 2020

Uh oh!

miss-islington commented Sep 7, 2020

Uh oh!

miss-islington commented Sep 7, 2020

Uh oh!

bedevere-bot commented Sep 7, 2020

Uh oh!

bedevere-bot commented Sep 7, 2020

Uh oh!

kodonnell commented Dec 6, 2021

Uh oh!

miss-islington commented Dec 6, 2021

Uh oh!

miss-islington commented Dec 6, 2021

Uh oh!

ethanfurman commented Dec 6, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

ArtemSBulgakov commented Jul 16, 2020•
edited by miss-islington
Loading

jaraco commented Sep 7, 2020•
edited
Loading

ethanfurman commented Dec 6, 2021•
edited
Loading