Signed-shift UB in zip-repair local-header read#640
Merged
Conversation
unzRepair parses the local file header of a damaged cache zip during repair. READ_32 combined two int-typed READ_16 halves as READ_16(adr) | (READ_16((adr)+2) << 16); when the high half has bit 15 set (>= 0x8000), shifting it left by 16 exceeds INT_MAX, which is signed overflow. UBSan aborts on the CRC/size fields of a header whose high 16-bit word has that bit set. Repair runs on hostile input (a corrupt or foreign new.zip). Cast the halves to uLong before the shift, matching the sibling minizip readers in unzip.c and zip.c. Closes #639 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
xroche
added a commit
that referenced
this pull request
Jul 18, 2026
Picks up the minizip READ_32 fix (#640) so the sanitize leg no longer aborts on the repair path. The Windows offline runner pins its expected skip set exactly, so a second Windows-skipped test (71_local-crange-repaircache) must be listed there too, otherwise it fails with "unexpected skips". Resolve tests/Makefile.am: keep 66/67/69 (merged) alongside this branch's 71. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
HTTrack's zip-repair reader (
unzRepairinsrc/minizip/mztools.c, Xavier's own 2004 file, not vendored from upstream minizip, so nothing to send upstream) built 32-bit header fields withREAD_32(adr) = READ_16(adr) | (READ_16((adr)+2) << 16).READ_16is anint, so a high half with bit 15 set shifts pastINT_MAX: signed overflow, and UBSan aborts on the CRC or size fields of a damaged header. Repair runs on hostile input, since it fires on a corrupt or foreignnew.zipcache. The fix casts the halves touLongbefore the shift, matching the genuinely-upstream siblingsunzip.candzip.cthat already read 32-bit fields that way.A new
zip-repair-shiftengine self-test drivesunzRepairover a header whose CRC high word has bit 15 set, wrapped bytests/01_zlib-repair-shift.test. On master it aborts withmztools.c:54:28: runtime error: left shift of 35560 by 16 places cannot be represented in type 'int'; with the fix it recovers the one entry cleanly. The test stands on its own and does not depend on #634's71_local-crange-repaircache.test, which trips the same UB only incidentally.#634's sanitize leg goes green once this lands (merge-order dependency).
Closes #639