zlib-ng / minizip-ng

Fork of the popular zip manipulation library found in the zlib distribution.
Other
1.24k stars 431 forks source link

Unzip AES support intentionally restricted to a specific vendor id? #11

Closed StevenChristy closed 10 years ago

StevenChristy commented 10 years ago

Nice work putting all this together. I encountered only one small issue trying to use this library. My 7zip AES encrypted zip file would not unzip properly. I traced the problem down to unz64local_GetCurrentFileInfoInternal.

Lines:

else if (headerid == 0x9901)
{
      /* Subtract size of AES field, since AES is handled internally */
      file_info.size_file_extra_internal += 2 + 2 + datasize;

      /* Verify version info */
      if (unz64local_getShort(&s->z_filefunc, s->filestream_with_CD, &uL) != UNZ_OK)
            err = UNZ_ERRNO;
      if (uL != 1)   // <---- this makes the unzip AES vendor specific. 7zip is putting 2 here.
            err = UNZ_ERRNO;
      if (unz64local_getByte(&s->z_filefunc, s->filestream_with_CD, &uL) != UNZ_OK)

The code tests if vendor id != 1 and if so it returns an error. Just my opinion, but I think the test should be removed entirely. Otherwise it should be noted that 7zip is compatible and uses 2 so the test could be changed to be uL != 1 && ul != 2.

If safety is the concern, one is probably better off making sure that file_info_internal.aes_encryption_mode and file_info_internal.aes_compression_method are being set to valid values rather than checking the vendor id. Just my opinion though.

StevenChristy commented 10 years ago

Nevermind, I just realized that I read this ( http://www.winzip.com/aes_info.htm ) wrong. 2 is not a vendor-specific ID its an indication that the CRC is calculated differently. Strangely removing the test worked, but I don't know what the consequences of that are yet so removing the test is not a good idea after all.