MSVC complains about "conversion from 'Py_ssize_t' to 'long', possible loss of data" in zipimport.c. header_offset is a Py_ssize_t but fseek() only takes a long. On 64bit Windows Py_ssize_t is a 64bit data type but long is still a 32bit data type.It's safe to assume that the header will be smaller than 2 GB for the next couple of years. :)

read_directory() uses fseek() and ftell() which don't support offset larger than LONG_MAX (2 GB on 32-bit system).  I don't know if it's an issue. What happens if the file is longer?"header_offset += arc_offset;" can overflow or not? This instuction looks weird.    header_position = ftell(fp);    ...    header_offset = get_long((unsigned char *)endof_central_dir + 16);    arc_offset = header_position - header_offset - header_size;    header_offset += arc_offset;If I computed correctly, the final line can be replaced with:    arc_offset = header_position - header_offset - header_size;    header_offset = header_position - header_size;(It is weird to reuse header_position for two different values, a new variable may be added.)Instead of checking that "header_offset > LONG_MAX", it may be safer to check that: - header_size >= 0 - header_offset >= 0 - header_offset + header_size <= LONG_MAX ---> header_offset <= LONG_MAX - header_size - arc_offset >= 0 ---> header_position >= header_offset + header_size - header_offset > 0 ---> header_position >= header_sizeIf all these values must be positive according to ZIP format, get_long() may be replaced with get_ulong() to simplify these checks.

See also zipfile.py which is probably more correct than zipimport.c: zipfile uses for example "L" format for struct.unpack (*unsigned* long) to decode header fields.

Yes, these fields are unsingned.

zipimport.c makes no attempt to support zip files larger than 2GiB or zip64 files.

Here is a work-in-progress patch.PyMarshal_ReadShortFromFile() and PyMarshal_ReadLongFromFile() are still wrong: new Unsigned version should be added to marshal.c. I don't know if a C cast to unsigned is enough because long can be larger than 32-bit (ex: on Linux 64-bit):#if SIZEOF_LONG > 4        /* Sign extension for 64-bit machines */        x |= -(x & 0x80000000L);#endifI didn't test my patch. Anyone interested to finish the patch?

comments added to the patch.

Ping.

Here is revised patch. It addresses Gregory's comments, uses properly integer types and converters for all values, and adds additional checks for integer overflows and ZIP file validity. As a side effect the performance can be increased due to less memory allocations and IO operations.

Sorry, the patch contained parts of the advanced patch that will be submitted in separate issue.

Synchronized with current sources.

Updated patch addresses Victor's comments and adds (mandatory now) parenthesis. Thank you Victor.

Serhiy Storchaka added the comment:> Updated patch addresses Victor's comments and adds (mandatory now) parenthesis. Thank you Victor.Do  you mean braces {...}?The new patch looks good to me, thanks for taking all my comments in account ;-)

New changeset687be1cbe587 by Serhiy Storchaka in branch '3.5':Issue#19883: Fixed possible integer overflows in zipimport.https://hg.python.org/cpython/rev/687be1cbe587New changesetf4631dc56ecf by Serhiy Storchaka in branch 'default':Issue#19883: Fixed possible integer overflows in zipimport.https://hg.python.org/cpython/rev/f4631dc56ecfNew changesetcebcd2fd3e1f by Serhiy Storchaka in branch '2.7':Issue#19883: Fixed possible integer overflows in zipimport.https://hg.python.org/cpython/rev/cebcd2fd3e1f

This seems to be causing an infinite loop in 2.7, in test_cmd_line_script.CmdLineTest.test_module_in_package_in_zipfile():test_module_in_package_in_zipfile (test.test_cmd_line_script.CmdLineTest) ... ^CTest suite interrupted by signal SIGINT.1 test omitted:    test_cmd_line_script[Exit 1][vadmium@localhost cpython]$ sudo strace -p 11412 2>&1 | headProcess 11412 attached - interrupt to quitlseek(3, 1024, SEEK_SET)                = 1024lseek(3, 1024, SEEK_SET)                = 1024lseek(3, 1024, SEEK_SET)                = 1024lseek(3, 1024, SEEK_SET)                = 1024lseek(3, 1024, SEEK_SET)                = 1024lseek(3, 1024, SEEK_SET)                = 1024lseek(3, 1024, SEEK_SET)                = 1024lseek(3, 1024, SEEK_SET)                = 1024lseek(3, 1024, SEEK_SET)                = 1024[vadmium@localhost cpython]$ ls -l /proc/11412/fdtotal 0lr-x------ 1 vadmium users 64 Jan 29 22:23 0 -> pipe:[1249749]l-wx------ 1 vadmium users 64 Jan 29 22:23 1 -> pipe:[1249750]l-wx------ 1 vadmium users 64 Jan 29 22:23 2 -> pipe:[1249750]lr-x------ 1 vadmium users 64 Jan 29 22:23 3 -> /tmp/tmpHMiuOm/test_zip.zip (deleted)[vadmium@localhost cpython]$ kill 11412

New changeset82ee3c24bb86 by Serhiy Storchaka in branch '2.7':Fixed an infinite loop in zipimport caused bycebcd2fd3e1f (issue#19883).https://hg.python.org/cpython/rev/82ee3c24bb86

Thank you Martin.