Fix Zip64 extra field offset computation in readCentralDirectoryFileHeader (#731)

Summary:

The Zip64 extended information extra field only includes entries for CDFH fields that overflow to 0xFFFFFFFF. The old code used hardcoded offsets (16 for header offset, 0 for compressed size), which assumed all preceding fields were always present. This caused corruption errors on zip files where only some fields used Zip64.

Fix by computing the offset dynamically based on which preceding CDFH fields actually overflowed. Also thread `opCtx` through the zip lexer for proper error reporting.

Differential Revision: D103841972

Author: terrelln

custom_parsers/pytorch_model_parser.c

@@ -172,7 +172,8 @@ static ZL_Report pytorchModelSegmenter(ZL_Segmenter* sctx)
     const ZL_GraphID functionGraph = customGraphs.graphids[0];
 
     ZS2_ZipLexer lexer;
-    ZL_ERR_IF_ERR(ZS2_ZipLexer_init(&lexer, ZL_Input_ptr(input), inputSize));
+    ZL_ERR_IF_ERR(ZS2_ZipLexer_init(
+            &lexer, ZL_Input_ptr(input), inputSize, ZL__errorContext.opCtx));
 
     const size_t nbFiles   = ZS2_ZipLexer_numFiles(&lexer);
     const size_t maxNbSegs = nbFiles * 4 + 2;

custom_parsers/tests/fuzz_zip_lexer.cpp

@@ -12,7 +12,7 @@ FUZZ(ZipLexerTest, FuzzLexer)
     std::array<ZS2_ZipToken, 10> tokens;
 
     ZS2_ZipLexer lexer;
-    auto report = ZS2_ZipLexer_init(&lexer, data.data(), data.size());
+    auto report = ZS2_ZipLexer_init(&lexer, data.data(), data.size(), NULL);
     if (ZL_isError(report)) {
         return;
     }

custom_parsers/tests/test_zip_lexer.cpp

@@ -110,6 +110,24 @@ std::array<uint8_t, 152> kTestZip64 = {
     0x00, 0x00, 0x53, 0x00, 0x00, 0x00, 0x00, 0x00
 };
 
+// Zip file where only the CDFH local file header offset is 0xFFFFFFFF.
+// Compressed and uncompressed sizes are normal, so the Zip64 extra field
+// contains only the 8-byte header offset. Exercises the dynamic offset
+// computation in readCentralDirectoryFileHeader (the old hardcoded offset
+// of 16 would fail here because the Zip64 extra field is only 8 bytes).
+std::array<uint8_t, 117> kTestZip64OffsetOnly = {
+    0x50, 0x4b, 0x03, 0x04, 0x0a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x05, 0x00, 0x00, 0x00, 0x05, 0x00,
+    0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x61, 0x68, 0x65, 0x6c, 0x6c, 0x6f,
+    0x50, 0x4b, 0x01, 0x02, 0x14, 0x00, 0x0a, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x05, 0x00, 0x00, 0x00,
+    0x05, 0x00, 0x00, 0x00, 0x01, 0x00, 0x0c, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff, 0x61, 0x01,
+    0x00, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x50,
+    0x4b, 0x05, 0x06, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x3b,
+    0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00, 0x00, 0x00,
+};
+
 std::array<uint8_t, 22> kTestEmptyZip = { 0x50, 0x4b, 0x05, 0x06, 0x00, 0x00,
                                           0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
                                           0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
@@ -120,7 +138,8 @@ void testZipLexer(
         std::string_view data,
         bool empty = false)
 {
-    ZL_REQUIRE_SUCCESS(ZS2_ZipLexer_init(lexer, data.data(), data.size()));
+    ZL_REQUIRE_SUCCESS(
+            ZS2_ZipLexer_init(lexer, data.data(), data.size(), NULL));
     const size_t numFiles = ZS2_ZipLexer_numFiles(lexer);
     if (empty) {
         ASSERT_EQ(numFiles, 0);
@@ -247,6 +266,15 @@ TEST(ZipLexerTest, Zip64)
             &lexer, { (const char*)kTestZip64.begin(), kTestZip64.size() });
 }
 
+TEST(ZipLexerTest, Zip64OffsetOnly)
+{
+    ZS2_ZipLexer lexer;
+    testZipLexer(
+            &lexer,
+            { (const char*)kTestZip64OffsetOnly.begin(),
+              kTestZip64OffsetOnly.size() });
+}
+
 TEST(ZipLexerTest, EmptyZip)
 {
     ZS2_ZipLexer lexer;

custom_parsers/zip_lexer.c

@@ -102,16 +102,17 @@ static bool hasZip64Info(const char* extraFieldPtr, size_t extraFieldLength)
  * Reads a 64-bit field from @p offsetTrusted (does not include the 4-byte
  * header).
  *
- * @p offsetTrusted An offset to read from that is guaranteed to be <= 20.
+ * @p offsetTrusted An offset to read from that is guaranteed to be <= 16.
  */
 static ZL_RESULT_OF(uint64_t) readZip64Info(
+        ZL_OperationContext* opCtx,
         const char* zip64InfoPtr,
         size_t zip64InfoSize,
         size_t offsetTrusted)
 {
-    ZL_RESULT_DECLARE_SCOPE(uint64_t, NULL);
+    ZL_RESULT_DECLARE_SCOPE(uint64_t, opCtx);
     const char* const zip64InfoEnd = zip64InfoPtr + zip64InfoSize;
-    ZL_ASSERT_LE(offsetTrusted, 20);
+    ZL_ASSERT_LE(offsetTrusted, 16);
     for (;;) {
         ZL_ERR_IF_LT(zip64InfoEnd - zip64InfoPtr, 4, corruption);
         const uint16_t id = ZL_readLE16(zip64InfoPtr);
@@ -133,12 +134,13 @@ static ZL_RESULT_OF(uint64_t) readZip64Info(
  * @returns The size of the CDFH entry or an error.
  */
 static ZL_Report readCentralDirectoryFileHeader(
+        ZL_OperationContext* opCtx,
         const char* cdfhPtr,
         const char* cdfhEnd,
         uint64_t* localFileHeaderOffset,
         uint64_t* compressedSize)
 {
-    ZL_RESULT_DECLARE_SCOPE_REPORT(NULL);
+    ZL_RESULT_DECLARE_SCOPE_REPORT(opCtx);
     // suppress -Wmaybe-uninitialized
     *localFileHeaderOffset = 0;
 
@@ -155,22 +157,33 @@ static ZL_Report readCentralDirectoryFileHeader(
     const char* const extraFieldPtr =
             cdfhPtr + kMinCentralDirectoryFileHeaderSize + filenameLength;
 
-    *localFileHeaderOffset = read32OrNeg1(cdfhPtr + 42);
-    if (*localFileHeaderOffset == (uint64_t)-1) {
-        ZL_TRY_LET(
-                uint64_t,
-                offset,
-                readZip64Info(extraFieldPtr, extraFieldLength, 16));
-        *localFileHeaderOffset = offset;
+    // Zip64 extra field stores values in order: uncompressed size,
+    // compressed size, header offset. Each field is only present when
+    // the corresponding CDFH field is 0xFFFFFFFF.
+    size_t zip64Offset = 0;
+    if (ZL_readLE32(cdfhPtr + 24) == (uint32_t)-1) {
+        zip64Offset += 8;
     }
 
     *compressedSize = read32OrNeg1(cdfhPtr + 20);
     if (*compressedSize == (uint64_t)-1) {
         ZL_TRY_LET(
                 uint64_t,
                 size,
-                readZip64Info(extraFieldPtr, extraFieldLength, 0));
+                readZip64Info(
+                        opCtx, extraFieldPtr, extraFieldLength, zip64Offset));
         *compressedSize = size;
+        zip64Offset += 8;
+    }
+
+    *localFileHeaderOffset = read32OrNeg1(cdfhPtr + 42);
+    if (*localFileHeaderOffset == (uint64_t)-1) {
+        ZL_TRY_LET(
+                uint64_t,
+                offset,
+                readZip64Info(
+                        opCtx, extraFieldPtr, extraFieldLength, zip64Offset));
+        *localFileHeaderOffset = offset;
     }
 
     return ZL_returnValue(cdfhLength);
@@ -235,6 +248,7 @@ static bool ZS2_ZipLexer_validateCentralDirectory(
     uint64_t localFileHeaderOffset;
     uint64_t compressedSize;
     if (ZL_isError(readCentralDirectoryFileHeader(
+                lexer->opCtx,
                 centralDirectoryPtr,
                 lexer->srcEnd,
                 &localFileHeaderOffset,
@@ -297,7 +311,7 @@ static ZL_Report ZS2_ZipLexer_findZipBegin(
         const char* maxRecordEnd,
         bool (*validate)(const ZS2_ZipLexer*, const char*, const char*))
 {
-    ZL_RESULT_DECLARE_SCOPE_REPORT(NULL);
+    ZL_RESULT_DECLARE_SCOPE_REPORT(lexer->opCtx);
     ZL_ERR_IF_LT(minRecordSize, 4, corruption);
     ZL_ERR_IF_GT(
             minRecordSize,
@@ -346,7 +360,7 @@ static ZL_Report ZS2_ZipLexer_findEOCD64(
         const char* eocdPtr,
         ZS2_ZipLexer_EndOfCentralDirectory* eocd)
 {
-    ZL_RESULT_DECLARE_SCOPE_REPORT(NULL);
+    ZL_RESULT_DECLARE_SCOPE_REPORT(lexer->opCtx);
     ZL_ASSERT(ZS2_ZipLexer_EndOfCentralDirectory_needZip64(eocd));
     ZL_ASSERT_GE(eocdPtr, lexer->zipBegin);
     ZL_ASSERT_LE(eocdPtr, lexer->srcEnd);
@@ -402,7 +416,7 @@ static ZL_Report ZS2_ZipLexer_findEOCD64(
  */
 static ZL_Report ZS2_ZipLexer_parseEOCD(ZS2_ZipLexer* lexer, size_t eocdOffset)
 {
-    ZL_RESULT_DECLARE_SCOPE_REPORT(NULL);
+    ZL_RESULT_DECLARE_SCOPE_REPORT(lexer->opCtx);
     ZL_ERR_IF_GT(
             eocdOffset, (size_t)(lexer->srcEnd - lexer->zipBegin), corruption);
     const char* const eocdPtr = lexer->zipBegin + eocdOffset;
@@ -487,6 +501,29 @@ static ZL_Report ZS2_ZipLexer_parseEOCD(ZS2_ZipLexer* lexer, size_t eocdOffset)
 
 static ZL_Report ZS2_ZipLexer_setFileState(ZS2_ZipLexer* lexer);
 
+static ZL_Report ZS2_ZipLexer_initWithEOCD(
+        ZS2_ZipLexer* lexer,
+        const void* src,
+        size_t srcSize,
+        size_t eocdOffset)
+{
+    ZL_RESULT_DECLARE_SCOPE_REPORT(lexer->opCtx);
+    lexer->zipBegin = src;
+    lexer->srcPtr   = src;
+    lexer->srcEnd   = lexer->zipBegin + srcSize;
+
+    memset(&lexer->fileState, 0, sizeof(lexer->fileState));
+
+    ZL_ERR_IF_ERR(ZS2_ZipLexer_parseEOCD(lexer, eocdOffset));
+
+    if (lexer->cdfhIdx < lexer->cdfhNum) {
+        // Proactively initialize the file state to catch more invalid zip files
+        // in the init() function.
+        ZL_ERR_IF_ERR(ZS2_ZipLexer_setFileState(lexer));
+    }
+    return ZL_returnSuccess();
+}
+
 static bool ZS2_ZipLexer_tryInit(
         ZS2_ZipLexer* lexer,
         const void* src,
@@ -507,10 +544,14 @@ static bool ZS2_ZipLexer_tryInit(
     return true;
 }
 
-ZL_Report
-ZS2_ZipLexer_init(ZS2_ZipLexer* lexer, const void* src, size_t srcSize)
+ZL_Report ZS2_ZipLexer_init(
+        ZS2_ZipLexer* lexer,
+        const void* src,
+        size_t srcSize,
+        ZL_OperationContext* opCtx)
 {
-    ZL_RESULT_DECLARE_SCOPE_REPORT(NULL);
+    lexer->opCtx = opCtx;
+    ZL_RESULT_DECLARE_SCOPE_REPORT(lexer->opCtx);
     const size_t minReverseOffset = kMinEndOfCentralDirectoryRecordSize;
     // Maximum allowed offset if there is no garbage at the end
     const size_t maxLegalReverseOffset =
@@ -542,29 +583,6 @@ ZS2_ZipLexer_init(ZS2_ZipLexer* lexer, const void* src, size_t srcSize)
     return ZL_returnSuccess();
 }
 
-ZL_Report ZS2_ZipLexer_initWithEOCD(
-        ZS2_ZipLexer* lexer,
-        const void* src,
-        size_t srcSize,
-        size_t eocdOffset)
-{
-    ZL_RESULT_DECLARE_SCOPE_REPORT(NULL);
-    lexer->zipBegin = src;
-    lexer->srcPtr   = src;
-    lexer->srcEnd   = lexer->zipBegin + srcSize;
-
-    memset(&lexer->fileState, 0, sizeof(lexer->fileState));
-
-    ZL_ERR_IF_ERR(ZS2_ZipLexer_parseEOCD(lexer, eocdOffset));
-
-    if (lexer->cdfhIdx < lexer->cdfhNum) {
-        // Proactively initialize the file state to catch more invalid zip files
-        // in the init() function.
-        ZL_ERR_IF_ERR(ZS2_ZipLexer_setFileState(lexer));
-    }
-    return ZL_returnSuccess();
-}
-
 /**
  * Emits an unknown token. This is used for bytes in the Zip file that are
  * otherwise unaccounted for. The Zip format, read loosely, allows for gaps in
@@ -578,7 +596,7 @@ static ZL_Report ZS2_ZipLexer_lexUnknown(
         ZS2_ZipToken* out,
         const char* nextPtr)
 {
-    ZL_RESULT_DECLARE_SCOPE_REPORT(NULL);
+    ZL_RESULT_DECLARE_SCOPE_REPORT(lexer->opCtx);
     ZL_ASSERT_GT(nextPtr, lexer->srcPtr);
     ZL_ASSERT_LE(nextPtr, lexer->srcEnd);
 
@@ -612,7 +630,7 @@ static ZL_Report ZS2_ZipLexer_lexSection(
         const char** sectionPtrPtr,
         size_t sectionSize)
 {
-    ZL_RESULT_DECLARE_SCOPE_REPORT(NULL);
+    ZL_RESULT_DECLARE_SCOPE_REPORT(lexer->opCtx);
     // Ensure the srcPtr is at the beginning of the section
     if (lexer->srcPtr < *sectionPtrPtr) {
         return ZS2_ZipLexer_lexUnknown(lexer, out, *sectionPtrPtr);
@@ -634,7 +652,7 @@ static ZL_Report ZS2_ZipLexer_lexSection(
 /// Handles emitting tokens for all sections after the files.
 static ZL_Report ZS2_ZipLexer_lexTail(ZS2_ZipLexer* lexer, ZS2_ZipToken* out)
 {
-    ZL_RESULT_DECLARE_SCOPE_REPORT(NULL);
+    ZL_RESULT_DECLARE_SCOPE_REPORT(lexer->opCtx);
     ZL_ERR_IF_NE(lexer->cdfhPtr, lexer->cdfhEnd, corruption);
 
     memset(out, 0, sizeof(*out));
@@ -691,12 +709,13 @@ static ZL_Report ZS2_ZipLexer_readNextCentralDirectoryFileHeader(
         uint64_t* localFileHeaderOffset,
         uint64_t* compressedSize)
 {
-    ZL_RESULT_DECLARE_SCOPE_REPORT(NULL);
+    ZL_RESULT_DECLARE_SCOPE_REPORT(lexer->opCtx);
     ZL_ASSERT_LT(lexer->cdfhIdx, lexer->cdfhNum);
     ZL_TRY_LET(
             size_t,
             cdfhLength,
             readCentralDirectoryFileHeader(
+                    lexer->opCtx,
                     lexer->cdfhPtr,
                     lexer->cdfhEnd,
                     localFileHeaderOffset,
@@ -723,7 +742,7 @@ static bool ZS2_ZipLexer_FileState_empty(
  */
 static ZL_Report ZS2_ZipLexer_setFileState(ZS2_ZipLexer* lexer)
 {
-    ZL_RESULT_DECLARE_SCOPE_REPORT(NULL);
+    ZL_RESULT_DECLARE_SCOPE_REPORT(lexer->opCtx);
     ZL_ASSERT(ZS2_ZipLexer_FileState_empty(&lexer->fileState));
 
     // Read fields from the CDFH. The compressed size is read from the CDFH,
@@ -796,7 +815,7 @@ static ZL_Report ZS2_ZipLexer_setFileState(ZS2_ZipLexer* lexer)
 
 static ZL_Report ZS2_ZipLexer_lexFile(ZS2_ZipLexer* lexer, ZS2_ZipToken* out)
 {
-    ZL_RESULT_DECLARE_SCOPE_REPORT(NULL);
+    ZL_RESULT_DECLARE_SCOPE_REPORT(lexer->opCtx);
     ZL_ASSERT_LE(lexer->cdfhIdx, lexer->cdfhNum);
     ZL_ASSERT_LE(lexer->cdfhPtr, lexer->cdfhEnd);
 
@@ -859,7 +878,7 @@ static ZL_Report ZS2_ZipLexer_lexOne(ZS2_ZipLexer* lexer, ZS2_ZipToken* out)
 ZL_Report
 ZS2_ZipLexer_lex(ZS2_ZipLexer* lexer, ZS2_ZipToken* out, size_t outCapacity)
 {
-    ZL_RESULT_DECLARE_SCOPE_REPORT(NULL);
+    ZL_RESULT_DECLARE_SCOPE_REPORT(lexer->opCtx);
     size_t entries;
     for (entries = 0; !ZS2_ZipLexer_finished(lexer) && entries < outCapacity;
          ++entries) {
@@ -886,6 +905,6 @@ size_t ZS2_ZipLexer_numFiles(const ZS2_ZipLexer* lexer)
 bool ZS2_isLikelyZipFile(const void* src, size_t srcSize)
 {
     ZS2_ZipLexer lexer;
-    const ZL_Report report = ZS2_ZipLexer_init(&lexer, src, srcSize);
+    const ZL_Report report = ZS2_ZipLexer_init(&lexer, src, srcSize, NULL);
     return !ZL_isError(report);
 }

custom_parsers/zip_lexer.h

@@ -45,25 +45,20 @@ typedef struct {
  * Initializes a Zip lexer on the given input buffer. The lexer allows for
  * garbage data before & after the zip file.
  *
+ * @param opCtx The operation context to use for error reporting or NULL.
+ *
  * @returns Success if the input buffer may be a valid zip file,
  *          or an error code if the input file is definitely not
  *          a supported zip file.
  *
  * @note This lexer supports all zip files whose central directory
  *       is listed in order of occurrence in the file.
  */
-ZL_Report
-ZS2_ZipLexer_init(ZS2_ZipLexer* lexer, const void* src, size_t srcSize);
-
-/**
- * Initializes a Zip lexer with a known offset to the EOCD.
- * @see ZS2_ZipLexer_init()
- */
-ZL_Report ZS2_ZipLexer_initWithEOCD(
+ZL_Report ZS2_ZipLexer_init(
         ZS2_ZipLexer* lexer,
         const void* src,
         size_t srcSize,
-        size_t eocdOffset);
+        ZL_OperationContext* opCtx);
 
 /**
  * Lexes the next @p outCapacity tokens from the input buffer.
@@ -139,6 +134,8 @@ struct ZS2_ZipLexer_s {
     uint32_t endOfCentralDirectoryRecordSize;
 
     ZS2_ZipLexer_FileState fileState;
+
+    ZL_OperationContext* opCtx;
 };
 
 ZL_END_C_DECLS