zipfile: consider rejecting duplicate unicode path extras
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 77.2k
- Forks
- 35.9k
- PR merge metrics
- PR metrics pending
Description
Feature or enhancement
Proposal:
Background
The ZIP format has several ways (and places) to encode a filename. One of them is in the "extra" fields, which appear on both local file and central directory entries.
The "Unicode Path" is one such extra, with identifier 0x7075. It conveys a UTF-8 variant of the entry's filename. Extras can occur multiple times in both the local file entries and the central directory; zipfile always selects the last one for a given identifier:
(Separately, zipfile only ever considers the extras as recorded in the CD entry. It does not attempt to reconcile/cross check them against the local file entries, which get skipped entirely.)
Problem
Unfortunately, this is differential prone, as different ZIP parsers have different precedent behaviors when a file has multiple Unicode Path extras.
For example, this produces a ZIP with two Unicode Path extras for the same member, and zipfile picks the last one (last.txt):
import io
import struct
import zlib
import zipfile
raw_name = b"raw.txt"
def unicode_path(name):
payload = struct.pack("<BI", 1, zlib.crc32(raw_name)) + name
return struct.pack("<HH", 0x7075, len(payload)) + payload
entry = zipfile.ZipInfo(raw_name.decode())
entry.extra = unicode_path(b"first.txt") + unicode_path(b"last.txt")
archive = io.BytesIO()
with zipfile.ZipFile(archive, "w") as z:
z.writestr(entry, b"payload")
archive.seek(0)
with zipfile.ZipFile(archive) as z:
print(z.namelist())
By contrast:
- libarchive and 7-Zip both select
first.txt - Go's
archive/zipand macOS'sunzipandzipselectraw.txt jar tfreportsraw.txt
Proposed solution
Unfortunately, the ZIP "spec" (I'm treating PKWARE's APPNOTE as canonical, but there are several) is not precise/clear about how to disambiguate/select between the various path forms. The spec stipulates that ZIP producers shouldn't generate a Unicode Path extra at all if bit 11 of the general-purpose bit flag is set, but it's not clear how consumers should behave when it is present.
Because there's no clear decision procedure here, IMO zipfile could be more conservative in what it accepts. In ascending order of breakage risk:
- Reject LF or CD entries that have more than one Unicode Path extra. This should be low-risk since honest encoders have no reason to encode multiple Unicode Path extras per entry. However, it still leaves open a differential with respect to the "base" filename within the entry itself.
- Reject any discrepancies between the Unicode Path and the entry's own filename. In other words, reject the ZIP if any potential filename for any entry is not byte-equivalent with all others for that entry. I suspect this would be fine for most real-world ZIPs, but it comes with some breakage risk on older files (which may not use ASCII or UTF-8 for the non-extra filename).
An option "1.5" between these two would be to add some kind of strict_paths: bool option to ZipFile, which would perform (2) when enabled. This could be done by default or not depending on breakage appetite 🙂
Misc
CCing @emmatyping and @sethmlarson as folks who I suspect would be interested in this 🙂.
CCing @zanieb as the original triager/discoverer of this.
(NB: I'm intentionally not filing this as a security item: while it's a differential, IMO it's a not particularly exploitable one because it only varies the filename, not the file contents.)
Has this already been discussed elsewhere?
This is a minor feature, which does not need previous discussion elsewhere
Links to previous discussion of this feature:
No response
Linked PRs
- gh-156641
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reviewing the Unicode Path extra handling in Lib/zipfile/init.py around the linked code, then inspect linked PR gh-156641. Compare the proposed rejection options and their compatibility implications; done means the chosen behavior is specified and covered by relevant zipfile tests.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100