zipfile: consider rejecting duplicate unicode path extras

[woodruffw]

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:

cpython/Lib/zipfile/__init__.py

Lines 586 to 629 in abdd7ae

	def _decodeExtra(self, filename_crc):
	# Try to decode the extra field.
	extra = self.extra
	unpack = struct.unpack
	while len(extra) >= 4:
	tp, ln = unpack('<HH', extra[:4])
	if ln+4 > len(extra):
	raise BadZipFile("Corrupt extra field %04x (size=%d)" % (tp, ln))
	if tp == 0x0001:
	data = extra[4:ln+4]
	# ZIP64 extension (large files and/or large archives)
	try:
	if self.file_size in (0xFFFF_FFFF_FFFF_FFFF, 0xFFFF_FFFF):
	field = "File size"
	self.file_size, = unpack('<Q', data[:8])
	data = data[8:]
	if self.compress_size == 0xFFFF_FFFF:
	field = "Compress size"
	self.compress_size, = unpack('<Q', data[:8])
	data = data[8:]
	if self.header_offset == 0xFFFF_FFFF:
	field = "Header offset"
	self.header_offset, = unpack('<Q', data[:8])
	except struct.error:
	raise BadZipFile(f"Corrupt zip64 extra field. "
	f"{field} not found.") from None
	elif tp == 0x7075:
	data = extra[4:ln+4]
	# Unicode Path Extra Field
	try:
	up_version, up_name_crc = unpack('<BL', data[:5])
	if up_version == 1 and up_name_crc == filename_crc:
	up_unicode_name = data[5:].decode('utf-8')
	if up_unicode_name:
	self.filename = _sanitize_filename(up_unicode_name)
	else:
	import warnings
	warnings.warn("Empty unicode path extra field (0x7075)", stacklevel=2)
	except struct.error as e:
	raise BadZipFile("Corrupt unicode path extra field (0x7075)") from e
	except UnicodeDecodeError as e:
	raise BadZipFile('Corrupt unicode path extra field (0x7075): invalid utf-8 bytes') from e
	extra = extra[ln+4:]

(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/zip and macOS's unzip and zip select raw.txt
• jar tf reports raw.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:

1. 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.
2. 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