PixarAnimationStudios/OpenUSD

USDZ and Zip Compliance

Opened this issue · 1 comments

Description of Issue

There is an unfortunate confluence of effects around USDZ's zip file.cpp that I would like to raise and suggest some solutions:

  1. The USDZ spec doesn't say that it is explicitly Zip32 , but the implementation is Zip32 only, and doesn't support Zip64.
  2. The USDZ spec says that it all USDZ files are compliant zip files.
  3. Zipfile.cpp doesn't read/validate the the central directory record and instead writes/reads the files linearly.

This means that you have the following issues:

  1. If you try and add a single file that is larger than 32bits, it will fail somewhat silently, as the values are invalid. See #3098
  2. You can add many files that are each under the 32bit limit but total over 32bits. This means that the central directory record (which is also 32bit) is out of spec as the offset values that point to the individual file offsets are invalid.

Therefore it's quite easy to make USDZ files that are technically corrupt zip files but USD will happily read again because it ignores the corrupt central directory listing..
However, it also means that you can easily corrupt the output file as well in silent ways, that USD will not read again.

I propose the following:

  1. We clarify in the documentation that it must be a Zip32. This will be part of the AOUSD specification language too.
  2. Zipfile.cpp should validate both the local file headers and the central directory record so we don't get invalid values
  3. We revisit zipfile.cpp to support more of the zip spec, starting with Zip64 support.

This is unlikely to be a security issue, but it is a data loss issue.

Steps to Reproduce

  1. Add a ton of files to a USDZ until you're over the 32bit limit.
  2. Check the central directory listing to see if the offsets point to the individual files.

Filed as internal issue #USD-10442