Repository navigation
Extract tar members through the tarfile data filter - #5337
inchang-ing wants to merge 2 commits into
Conversation
unpack_tarfile extracted through three private tarfile APIs (_extract_member, _getmember and the chown override), so none of the PEP 706 hardening ran: archive-supplied setuid/setgid/sticky bits and ownership reached the filesystem, and link resolution depended on an API with no stability guarantee. Resolve links through the public getmembers() listing with the same exact-name, last-occurrence-wins semantics, and run the resolved member through tarfile.data_filter before it is written, picking up the stdlib's sanitized copy (high mode bits stripped, ownership cleared, special files rejected). The chown suppression is retained only for Pythons without the filter backport. Fixes pypa#5328
|
Tick the box to add this pull request to the merge queue (same as
|
| # and take its sanitized copy (high mode bits stripped, | ||
| # ownership cleared, special files rejected) before the | ||
| # member is written anywhere. | ||
| member = _DATA_FILTER(member, extract_dir) |
There was a problem hiding this comment.
With a hard link ok to a later member ../evil, this line raises tarfile.OutsideDestinationError where main raises UnsafeMember. Callers catching UnsafeMember or DistutilsError miss this abort.
The PEP 706 data filter raises tarfile.OutsideDestinationError for a member whose (link-resolved) name escapes the destination, whereas the pre-filter code raised UnsafeMember through _resolve_dest. Callers catching UnsafeMember or DistutilsError would miss the abort entirely, so the extraction driver's exception contract silently changed. Translate the filter's outside-destination and absolute-path errors into UnsafeMember, keeping the contract identical to the pre-filter behavior. A parameterized regression test pins both hard links and symlinks. Reported-by: jamalkamaladdin
|
Good catch - fixed in Reproduced it: for a hard link
Covered by |
|
Hi maintainers, gentle ping on #5337 — Extract tar members through the tarfile data filter. It's a small, self-contained fix with passing tests. Would appreciate a review when you have a moment. Thanks for maintaining pypa/setuptools! |
Fixes #5328
Problem
setuptools.archive_util.unpack_tarfileextracted through three privatetarfileAPIs, so none of the PEP 706 hardening ever ran:_extract_member(member, final_dst)bypassed the extraction filter entirely, letting archive-supplied setuid/setgid/sticky bits and ownership reach the filesystem;tar_obj.chown = lambda *args: Nonewas a hand-rolled substitute for one piece of what the filter does properly;tar_obj._getmember(linkpath)resolved link targets through an API with no stability guarantee.The containment check from #5325 re-implemented one filter protection by hand; this change lets the stdlib own the rest.
Approach (option 1 from the issue)
getmembers()listing with the same semantics as the old private call: exact name match, last occurrence wins,Nonewhen absent. Notably the exact match is preserved deliberately —getmember()would not be equivalent, since it rstrips trailing slashes from the query (tarfile already strips them from parsed member names, so exact matching is what reproduces today's behavior).tarfile.data_filter(member, extract_dir), and its sanitized copy is extracted to the possibly-redirected destination fromprogress_filter. This keeps the documentedprogress_filtersemantics (including redirection) while applying the stdlib's validation and sanitization (high mode bits stripped, ownership cleared, special files rejected).chownsuppression is retained only for Pythons lacking the filter (early 3.10/3.11 micro releases without the security backport), where behavior is unchanged from before.requires-python >= 3.10is satisfied for the filter itself by the 3.10.12+/3.11.4+ backports; thegetattrguard covers anything older than those micros.Testing
test_iter_open_tar_applies_data_filter(cross-platform, at theTarInfolevel): a setuid member comes out of_iter_open_tarwith high mode bits stripped and ownership cleared.test_unpack_tarfile_strips_high_mode_bits(POSIX): end-to-end, the extracted file carries no setuid bit and stays executable.test_unpack_tarfile_resolves_links_to_targets: hardlink members are materialized as copies of their archive-relative targets through the public lookup (link resolution previously had no coverage).test_archive_util.py(17 tests) andtest_dist_info.py(which unpacks a real wheel throughunpack_archive) pass:53 passed, 2 skipped, 1 xpassed; the skip/xpass are pre-existing (symlink support on Windows, UnicodeEncodeError in archive_util #710/sandbox.run_setup incorrectly sets__file__when setup_script is Unicode on Python 2 #712).Disclosure
This PR was prepared with AI assistance (ZCode/GLM, orchestrated via WorkBuddy).