This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Unable to read zip file with extra
Type: crash Stage: resolved
Components: Library (Lib) Versions: Python 3.7
process
Status: closed Resolution: not a bug
Dependencies: Superseder:
Assigned To: Nosy List: Andreas.Gäer, altendky, serhiy.storchaka, v2m, xtreak
Priority: normal Keywords:

Created on 2018-09-07 14:41 by altendky, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 9107 closed v2m, 2018-09-07 21:42
Messages (9)
msg324743 - (view) Author: Kyle Altendorf (altendky) * Date: 2018-09-07 14:41
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.


----
    altendky@lt:~/twisted$ cat ../z.py
    import sys
    import zipfile

    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$ venv36/bin/python ../z.py 36.zip
    3.6.6 (default, Jul 24 2018, 16:23:12)
    [GCC 6.3.0 20170516]
    36.zip

----
    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)

----
    altendky@lt:~/twisted$ cat ../z.py
    import sys
    import zipfile

    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$ venv36/bin/python ../z.py 37.zip
    3.6.6 (default, Jul 24 2018, 16:23:12)
    [GCC 6.3.0 20170516]
    37.zip

----
    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@lt:~/twisted$ sha256sum 36.zip
    0f54bd6ab84facfeefc2c38f12c30eb84101b3be3d91f8826f6fa36e73b86cb6  36.zip

----
    altendky@lt:~/twisted$ sha256sum 37.zip
    0f54bd6ab84facfeefc2c38f12c30eb84101b3be3d91f8826f6fa36e73b86cb6  37.zip
msg324766 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python committer) Date: 2018-09-07 15:47
It's also reproducible on master. git blame tells me this is introduced with feccdb2a249a71be330765be77dee57121866779 (https://bugs.python.org/issue29774)

➜  cpython git:(master)
commit feccdb2a249a71be330765be77dee57121866779
Author: Serhiy Storchaka <storchaka@gmail.com>
Date:   Thu Mar 9 18:34:03 2017 +0200

    bpo-29774: Improve error reporting for corrupted extra field in ZIP file. (#583)

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 feccdb2a249a71be330765be77dee57121866779

➜  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 feccdb2a249a71be330765be77dee57121866779~1

➜  cpython git:(master) ✗ git checkout feccdb2a249a71be330765be77dee57121866779~1 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
➜  cpython git:(master) ✗ gsha256sum 36.zip
0f54bd6ab84facfeefc2c38f12c30eb84101b3be3d91f8826f6fa36e73b86cb6  36.zip


Hope this helps. 

I am adding Serhiy who might have a better explanation. @Serhiy Feel free to unassign yourself if this is irrelevant.

Thanks
msg324790 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-09-07 20:40
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.
msg324791 - (view) Author: Kyle Altendorf (altendky) * Date: 2018-09-07 20:57
Turns out the docs do document this.  My apologies.

https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT

   4.3.11  Archive extra data record: 

        archive extra data signature    4 bytes  (0x08064b50)
        extra field length              4 bytes
        extra field data                (variable size)

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)?
msg324797 - (view) Author: Kyle Altendorf (altendky) * Date: 2018-09-07 21:21
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
    --------------------------
    
       4.5.1 In order to allow different programs and different types
       of information to be stored in the 'extra' field in .ZIP
       files, the following structure MUST be used for all
       programs storing data in this field:
    
           header1+data1 + header2+data2 . . .
    
       Each header should consist of:
    
           Header ID - 2 bytes
           Data Size - 2 bytes
    
       Note: all fields stored in Intel low-byte/high-byte order.


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')

----
    altendky@lt:~/twisted$ python3.7 ../z.py 37.py
    3.7.0 (default, Jul  7 2018, 15:49:24)
    [GCC 6.3.0 20170516]
    37.py
    
    
     --- trying endianness: little, additional_size: 0, header_element_bytes: 2
    success
    
    
     --- trying endianness: little, additional_size: 0, header_element_bytes: 4
    Corrupt extra field 6568 (size=27756)
    
    
     --- trying endianness: little, additional_size: 4, header_element_bytes: 2
    Corrupt extra field 002a (size=71)
    
    
     --- trying endianness: little, additional_size: 4, header_element_bytes: 4
    Corrupt extra field 6568 (size=27756)
    
    
     --- trying endianness: little, additional_size: 8, header_element_bytes: 2
    Corrupt extra field 002a (size=75)
    
    
     --- trying endianness: little, additional_size: 8, header_element_bytes: 4
    Corrupt extra field 6568 (size=27756)
    
    
     --- trying endianness: big, additional_size: 0, header_element_bytes: 2
    Corrupt extra field 2a00 (size=17152)
    
    
     --- trying endianness: big, additional_size: 0, header_element_bytes: 4
    Corrupt extra field 0000 (size=10752)
    
    
     --- trying endianness: big, additional_size: 4, header_element_bytes: 2
    Corrupt extra field 2a00 (size=18176)
    
    
     --- trying endianness: big, additional_size: 4, header_element_bytes: 4
    Corrupt extra field 0000 (size=10752)
    
    
     --- trying endianness: big, additional_size: 8, header_element_bytes: 2
    Corrupt extra field 2a00 (size=19200)
    
    
     --- trying endianness: big, additional_size: 8, header_element_bytes: 4
    Corrupt extra field 0000 (size=10752)
msg324799 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-09-07 21:39
Right, the correct section is 4.5. All integers in ZIP files are unsigned and in the little endian format.
msg325119 - (view) Author: Vladimir Matveev (v2m) * Date: 2018-09-12 04:33
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.
msg325132 - (view) Author: Kyle Altendorf (altendky) * Date: 2018-09-12 11:24
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.
msg346518 - (view) Author: Andreas Gäer (Andreas.Gäer) Date: 2019-06-25 12:21
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__?
History
Date User Action Args
2022-04-11 14:59:05adminsetgithub: 78787
2019-06-25 12:21:32Andreas.Gäersetnosy: + Andreas.Gäer
messages: + msg346518
2018-09-12 11:24:30altendkysetmessages: + msg325132
2018-09-12 04:33:37v2msetnosy: + v2m
messages: + msg325119
2018-09-07 21:42:31v2msetpull_requests: + pull_request8561
2018-09-07 21:39:40serhiy.storchakasetmessages: + msg324799
2018-09-07 21:21:22altendkysetmessages: + msg324797
2018-09-07 20:57:12altendkysetmessages: + msg324791
2018-09-07 20:40:31serhiy.storchakasetstatus: open -> closed
resolution: not a bug
messages: + msg324790

stage: resolved
2018-09-07 15:47:56xtreaksetnosy: + serhiy.storchaka
messages: + msg324766
2018-09-07 15:33:06xtreaksetnosy: + xtreak
2018-09-07 14:41:59altendkycreate