Unable to read zip file with extra #78787
Comments
|
This was first found over in Twisted tests. We probably aren't too terribly worried about it but I wanted to report here anyways. https://twistedmatrix.com/trac/ticket/9525 Both 3.6 and 3.7 write the same file (sha at the end) based on the script in the first snippet. 3.6 can read both files, 3.7 can't read either. ---- print(sys.version)
fn = sys.argv[1]
print(fn)
with zipfile.ZipFile(fn, 'w') as zf:
zi = zipfile.ZipInfo("0")
zi.extra = b"hello, extra"
zf.writestr(zi, b"the real data")
zipfile.ZipFile(fn)---- altendky@lt:~/twisted$ venv37/bin/python ../z.py 37.zip
3.7.0 (default, Jul 7 2018, 15:49:24)
[GCC 6.3.0 20170516]
37.zip
Traceback (most recent call last):
File "../z.py", line 14, in <module>
zipfile.ZipFile(fn)
File "/home/altendky/.pyenv/versions/3.7.0/lib/python3.7/zipfile.py", line 1200, in __init__
self._RealGetContents()
File "/home/altendky/.pyenv/versions/3.7.0/lib/python3.7/zipfile.py", line 1323, in _RealGetContents
x._decodeExtra()
File "/home/altendky/.pyenv/versions/3.7.0/lib/python3.7/zipfile.py", line 440, in _decodeExtra
raise BadZipFile("Corrupt extra field %04x (size=%d)" % (tp, ln))
zipfile.BadZipFile: Corrupt extra field 6568 (size=27756) print(sys.version)
fn = sys.argv[1]
print(fn) zipfile.ZipFile(fn)---- altendky@lt:~/twisted$ venv37/bin/python ../z.py 36.zip
3.7.0 (default, Jul 7 2018, 15:49:24)
[GCC 6.3.0 20170516]
36.zip
Traceback (most recent call last):
File "../z.py", line 14, in <module>
zipfile.ZipFile(fn)
File "/home/altendky/.pyenv/versions/3.7.0/lib/python3.7/zipfile.py", line 1200, in __init__
self._RealGetContents()
File "/home/altendky/.pyenv/versions/3.7.0/lib/python3.7/zipfile.py", line 1323, in _RealGetContents
x._decodeExtra()
File "/home/altendky/.pyenv/versions/3.7.0/lib/python3.7/zipfile.py", line 440, in _decodeExtra
raise BadZipFile("Corrupt extra field %04x (size=%d)" % (tp, ln))
zipfile.BadZipFile: Corrupt extra field 6568 (size=27756)---- |
altendky
mannequin
added
3.7 (EOL)
|
It's also reproducible on master. git blame tells me this is introduced with feccdb2 (https://bugs.python.org/issue29774) ➜ cpython git:(master) diff --git a/Lib/zipfile.py b/Lib/zipfile.py
index b5c16dbc12..8a19ca246b 100644
--- a/Lib/zipfile.py
+++ b/Lib/zipfile.py
@@ -438,7 +438,9 @@ class ZipInfo (object):
unpack = struct.unpack
while len(extra) >= 4:
tp, ln = unpack('<HH', extra[:4])
- if tp == 1:
+ if ln+4 > len(extra):
+ raise BadZipFile("Corrupt extra field %04x (size=%d)" % (tp, ln))
+ if tp == 0x0001:
if ln >= 24:
counts = unpack('<QQQ', extra[4:28])
elif ln == 16:# Master branch ➜ cpython git:(master) ./python.exe
Python 3.8.0a0 (heads/master:874809ea38, Sep 7 2018, 21:03:18)
[Clang 7.0.2 (clang-700.1.81)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>>
➜ cpython git:(master) ./python.exe ../backups/bpo34606.py 36.zip
3.8.0a0 (heads/master:874809ea38, Sep 7 2018, 21:03:18)
[Clang 7.0.2 (clang-700.1.81)]
36.zip
Traceback (most recent call last):
File "../backups/bpo34606.py", line 14, in <module>
zipfile.ZipFile(fn)
File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/zipfile.py", line 1204, in __init__
self._RealGetContents()
File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/zipfile.py", line 1327, in _RealGetContents
x._decodeExtra()
File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/zipfile.py", line 440, in _decodeExtra
raise BadZipFile("Corrupt extra field %04x (size=%d)" % (tp, ln))
zipfile.BadZipFile: Corrupt extra field 6568 (size=27756)
➜ cpython git:(master) ✗ rm 36.zip# checkout feccdb2 ➜ cpython git:(master) git checkout feccdb2a249a71be330765be77dee57121866779 Lib/zipfile.py
➜ cpython git:(master) ✗ ./python.exe ../backups/bpo34606.py 36.zip
3.8.0a0 (heads/master:874809ea38, Sep 7 2018, 21:03:18)
[Clang 7.0.2 (clang-700.1.81)]
36.zip
Traceback (most recent call last):
File "../backups/bpo34606.py", line 14, in <module>
zipfile.ZipFile(fn)
File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/zipfile.py", line 1105, in __init__
self._RealGetContents()
File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/zipfile.py", line 1230, in _RealGetContents
x._decodeExtra()
File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/zipfile.py", line 442, in _decodeExtra
raise BadZipFile("Corrupt extra field %04x (size=%d)" % (tp, ln))
zipfile.BadZipFile: Corrupt extra field 6568 (size=27756)
➜ cpython git:(master) ✗ rm 36.zip# checkout feccdb2~1 ➜ cpython git:(master) ✗ git checkout feccdb2~1 Lib/zipfile.py Hope this helps. I am adding Serhiy who might have a better explanation. @serhiy Feel free to unassign yourself if this is irrelevant. Thanks |
|
The value of the extra attribute is not an arbitrary bytes object. It is a sequence of blocks in the following format: 16-bit identifier followed by 16-bit size of the block followed by the specified amount of bytes. Your example creates invalid ZIP file. |
|
Turns out the docs do document this. My apologies. https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT 4.3.11 Archive extra data record: Aside from the discrepancy between 16-bits and 4 bytes, it seems like something should happen, even if it's something other than 'fixing' the code to handle the malformed data. Isn't it a bug for zipfile to create a non-compliant file? Shouldn't it either check or provide an interface by which a compliant file could sensibly be created? It doesn't seem great to just expect users to rewrite this each time they call. (42).to_bytes(4, 'little') + len(data).to_bytes(4, 'little') + data or, should it be 'big'? and would it be (len(data) + 4 + 4)? |
|
Python 3.7 works with 2-byte elements, I managed to find the wrong section in the doc-linked docs. 4.5 Extensible data fields And the test showing it working. ---- import sys
import zipfile
print(sys.version)
fn = sys.argv[1]
print(fn)
options = {
'endianness': ('little', 'big'),
'header_element_bytes': (2, 4),
'additional_size': (0, 4, 4 + 4),
}
for endianness in options['endianness']:
for additional_size in options['additional_size']:
for header_element_bytes in options['header_element_bytes']:
print('\n\n --- trying endianness: {}, additional_size: {}, header_element_bytes: {}'.format(endianness, additional_size, header_element_bytes))
with zipfile.ZipFile(fn, 'w') as zf:
zi = zipfile.ZipInfo("0")
extra_data = b"hello, extra, and some more just to make it longer and such so yeah"
zi.extra = (
(42).to_bytes(header_element_bytes, endianness)
+ (len(extra_data) + additional_size).to_bytes(header_element_bytes, endianness)
+ extra_data
)
zf.writestr(zi, b"the real data")
try:
zipfile.ZipFile(fn)
except Exception as e:
print(e)
else:
print('success') |
|
Right, the correct section is 4.5. All integers in ZIP files are unsigned and in the little endian format. |
|
In this particular case looks like a crux of the problem was in the fact that compression encodes extra fields only if either zip64 is set or length of the field is larger than threshold but decompression always tries to decode it. Attached PR switches decoding to be conditioned on the presence of zip64 end of central directory record. |
|
Vladimir, if compression were the cause wouldn't the extra bytes I added (signature and length) not have any effect? If you've found an issue it seems like it would be a different one than I was triggering. |
|
I'm a little bit puzzled that this bug is closed as "not a bug" despite the fact that all versions of python allow the user to create "invalid" ZIP files and all version up to 3.7 are able to read those "invalid" files. Currently we're stuck with a lot of already created ZIP files that cannot be read or fixed after an update to 3.7. Would it be an option to make decoding of extra data optional eg. with an extra keyword arg to __init__? |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: