Fix unchecked Minizip extra-field lengths

contrib/minizip/test/CMakeLists.txt

@@ -1,4 +1,18 @@
 # if we are built from with zlib, use this path's)
+
+set(MINIZIP_TEST_LIBRARY MINIZIP::minizipstatic)
+
+add_executable(
+    ${ZLIB_CONTRIB_PREFIX}minizip_extra_field_bounds_test
+    extra_field_bounds.c)
+target_link_libraries(
+    ${ZLIB_CONTRIB_PREFIX}minizip_extra_field_bounds_test
+    PRIVATE ${MINIZIP_TEST_LIBRARY})
+add_test(
+    NAME ${ZLIB_CONTRIB_PREFIX}minizip_extra_field_bounds_test
+    COMMAND ${ZLIB_CONTRIB_PREFIX}minizip_extra_field_bounds_test
+    WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
+
 if(ZLIB_MINIZIP_INSTALL)
     if(NOT DEFINED ZLIB_BUILD_MINIZIP)
         set(WORK_DIR ${CMAKE_CURRENT_BINARY_DIR})

contrib/minizip/test/extra_field_bounds.c

@@ -0,0 +1,38 @@
+#include <stdio.h>
+#include <string.h>
+
+#include "zip.h"
+
+static int expect_rejected(char *data, int data_len) {
+    char original[8];
+    int original_len = data_len;
+    int result;
+
+    memcpy(original, data, (size_t)data_len);
+    result = zipRemoveExtraInfoBlock(data, &data_len, 0x1234);
+    if (result != ZIP_PARAMERROR || data_len != original_len ||
+        memcmp(data, original, (size_t)original_len) != 0) {
+        fprintf(stderr, "malformed extra field was not rejected safely\n");
+        return 1;
+    }
+    return 0;
+}
+
+int main(void) {
+    char short_header[] = {0x34, 0x12, 0x00};
+    char truncated_block[] = {0x34, 0x12, 0x01, 0x00};
+    char valid_block[] = {0x34, 0x12, 0x02, 0x00, (char)0xab, (char)0xcd};
+    int valid_len = (int)sizeof(valid_block);
+
+    if (expect_rejected(short_header, (int)sizeof(short_header)) != 0 ||
+        expect_rejected(truncated_block, (int)sizeof(truncated_block)) != 0)
+        return 1;
+
+    if (zipRemoveExtraInfoBlock(valid_block, &valid_len, 0x1234) != ZIP_OK ||
+        valid_len != 0) {
+        fprintf(stderr, "valid extra field was not removed\n");
+        return 1;
+    }
+
+    return 0;
+}

contrib/minizip/zip.c

@@ -2199,32 +2199,51 @@ extern int ZEXPORT zipRemoveExtraInfoBlock(char* pData, int* dataLen, short sHea
   int size = 0;
   char* pNewHeader;
   char* pTmp;
-  short header;
-  short dataSize;
+  char* end;
+  unsigned short header;
+  unsigned short dataSize;
+  unsigned int blockSize;
+  size_t remaining;
 
   int retVal = ZIP_OK;
 
   if(pData == NULL || dataLen == NULL || *dataLen < 4)
     return ZIP_PARAMERROR;
 
   pNewHeader = (char*)ALLOC((unsigned)*dataLen);
+  if (pNewHeader == NULL)
+    return ZIP_INTERNALERROR;
   pTmp = pNewHeader;
+  end = pData + *dataLen;
 
-  while(p < (pData + *dataLen))
+  while(p < end)
   {
-    header = *(short*)p;
-    dataSize = *(((short*)p)+1);
+    remaining = (size_t)(end - p);
+    if (remaining < 4)
+    {
+      retVal = ZIP_PARAMERROR;
+      goto cleanup;
+    }
+
+    header = *(unsigned short*)p;
+    dataSize = *(((unsigned short*)p)+1);
+    blockSize = (unsigned int)dataSize + 4;
+    if (blockSize > remaining)
+    {
+      retVal = ZIP_PARAMERROR;
+      goto cleanup;
+    }
 
     if( header == sHeader ) /* Header found. */
     {
-      p += dataSize + 4; /* skip it. do not copy to temp buffer */
+      p += blockSize; /* skip it. do not copy to temp buffer */
     }
     else
     {
       /* Extra Info block should not be removed, So copy it to the temp buffer. */
-      memcpy(pTmp, p, dataSize + 4);
-      p += dataSize + 4;
-      size += dataSize + 4;
+      memcpy(pTmp, p, blockSize);
+      p += blockSize;
+      size += blockSize;
     }
 
   }
@@ -2246,6 +2265,7 @@ extern int ZEXPORT zipRemoveExtraInfoBlock(char* pData, int* dataLen, short sHea
   else
     retVal = ZIP_ERRNO;
 
+cleanup:
   free(pNewHeader);
 
   return retVal;