Info-ZIP project

Confirmed. Okay w/me to close it.

This should be fixed in Zip 3.0 as released. Please confirm this bug can
be closed. Thanks!

Thank you for your efforts, Ed. I just wanted to comment that the impact
is more than just the extra fields getting written to the wrong location.
A memcpy() is done which violates the bounds of malloc'd() memory space,
which is always bad news, plus it's in generic code (not specific to
MinGW). That's why I wanted to get this in the release. I have done brief
testing with the fix. Like I said, it resolved my problem. Now when I
create a large archive and then add another large set of files to it (8 GiB
-> 11 GiB), it seems to run flawlessly and tests perfectly with "unzip -t".

It seems the impact of this bug is if extra fields are written after the
Zip64 extra field they get written to a slightly wrong location in the
central directory entry extra field block. This can cause the crashes that
Will is seeing in specific cases. Note that the order of extra fields in
an existing extra field block is not determined, as well as where an
existing Zip64 extra field might be in the block, so without digging in to
the structure in the existing archive it's hard to tell when the bug might
happen. Given the possibility of archive corruption, it seems this change
needs to be made in Zip 3.0 to fix the bug.

I've traced the change through and it looks correct to me. I'd appreciate
others confirming this. Making this change may delay releasing Zip 3.0 for
a couple days as a couple tests need to be done.

It looks like add_local_zip64_extra_field(), which does the same thing but
for the local header, approaches this slightly differently to avoid this
problem. Someone might compare the two.

If some testing with creating and updating Zip64 archives can be done,
that will speed getting Zip 3.0 out. This should be the last change before
release.

Here is the updated code in zipfile.c: (starting at line 1183)

else
{
/* get the old Zip64 extra field out and add new */
oldefsize = usTemp + ZIP_EF_HEADER_SIZE;
if ((pTemp = (char *) malloc(pZipListEntry->cext - oldefsize +
efsize)) == NULL) {
return ZE_MEM;
}
len = (extent)(pExtraFieldPtr - pZipListEntry->cextra);
memcpy(pTemp, pZipListEntry->cextra, len);
memcpy(pTemp + len, pExtraFieldPtr + oldefsize,
pZipListEntry->cext - oldefsize - len);
pZipListEntry->cext -= oldefsize;
pExtraFieldPtr = pTemp + pZipListEntry->cext;
pZipListEntry->cext += efsize;
free(pZipListEntry->cextra);
pZipListEntry->cextra = pTemp;
}

Thanks all.

"//" is _not_ the most portable C comment delimiter.

I think I found the bug. It's due to a memcpy() that writes outside the
bounds of memory that was just allocated with malloc. The fix below
eliminated the abort, and the resultant test archive passed the "unzip -t"
test.

Fix: Lines 1192 and 1193 in zipfile.c in the function
add_central_zip64_extra_field():

len = pZipListEntry->cext - oldefsize - len;
memcpy(pTemp + len, pExtraFieldPtr + oldefsize, len);

... should be replaced by...

// len = pZipListEntry->cext - oldefsize - len; (or just delete
this line)
memcpy(pTemp + len, pExtraFieldPtr + oldefsize,
pZipListEntry->cext - oldefsize - len);