Skip to content

Commit

Permalink
Fix corruption of large files when zip64 is used
Browse files Browse the repository at this point in the history
Previously the code did not account for the fact that the initial stub local
header (with uncompressed and compressed sizes set to 0) could not serve for
correct estimation of the final local header size due to the fact that the
local header size was determined by the uncompressed and compressed sizes of
the corresponding data, which are only known after streaming of the data.
These sizes dictated whether or not a zip64 extra field entry should be
included in the header or not. Thus, before this fix there would be cases of
corruption where the final (longer) local header written by seeking back to
the beginning of the initial stub local header after the data had been
streamed would overwrite the beginning of the data.

This is fixed by

* always writing a zip64 entry in local headers, which does not violate the
  spec and will be safely ignored in the case of smaller entries, and
* respecting the spec more precisely where it says that whenever there is a
  zip64 extra field entry in a local header both uncompressed and compressed
  sizes must always be written.

This is deemed safe because the only source of size variation for local
headers is the uncompressed and compressed sizes of the corresponding data.
  • Loading branch information
mrkkrp committed Apr 20, 2024
1 parent 63a3553 commit da5df36
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 8 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## Zip 2.0.1

* Fixed corruption of large entries when zip64 is used. [Issue
111](https://github.com/mrkkrp/zip/issues/111).

## Zip 2.0.0

* Unified `BZip2Unsupported` and `ZstdUnsupported` into a single data
Expand Down
20 changes: 13 additions & 7 deletions Codec/Archive/Zip/Internal.hs
Original file line number Diff line number Diff line change
Expand Up @@ -779,11 +779,16 @@ makeZip64ExtraField ::
-- | Resulting representation
ByteString
makeZip64ExtraField headerType Zip64ExtraField {..} = runPut $ do
when (headerType == LocalHeader || z64efUncompressedSize >= ffffffff) $
putWord64le (fromIntegral z64efUncompressedSize) -- uncompressed size
when (headerType == LocalHeader || z64efCompressedSize >= ffffffff) $
putWord64le (fromIntegral z64efCompressedSize) -- compressed size
when (headerType == CentralDirHeader && z64efOffset >= ffffffff) $
case headerType of
LocalHeader -> do
putWord64le (fromIntegral z64efUncompressedSize) -- uncompressed size
putWord64le (fromIntegral z64efCompressedSize) -- compressed size
CentralDirHeader -> do
when (z64efUncompressedSize >= ffffffff) $
putWord64le (fromIntegral z64efUncompressedSize) -- uncompressed size
when (z64efCompressedSize >= ffffffff) $
putWord64le (fromIntegral z64efCompressedSize) -- compressed size
when (z64efOffset >= ffffffff) $
putWord64le (fromIntegral z64efOffset) -- offset of local file header

-- | Create 'ByteString' representing an extra field.
Expand All @@ -810,7 +815,8 @@ putHeader ::
EntryDescription ->
Put
putHeader headerType s entry@EntryDescription {..} = do
let isCentralDirHeader = headerType == CentralDirHeader
let isLocalHeader = headerType == LocalHeader
isCentralDirHeader = headerType == CentralDirHeader
putWord32le (bool 0x04034b50 0x02014b50 isCentralDirHeader)
-- ↑ local/central file header signature
when isCentralDirHeader $
Expand Down Expand Up @@ -842,7 +848,7 @@ putHeader headerType s entry@EntryDescription {..} = do
}
extraField =
B.take 0xffff . runPut . putExtraField $
if needsZip64 entry
if needsZip64 entry || isLocalHeader
then M.insert 1 zip64ef edExtraField
else edExtraField
putWord16le (fromIntegral $ B.length extraField) -- extra field length
Expand Down
2 changes: 1 addition & 1 deletion tests/Main.hs
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ checkEntrySpec = do
it "does not pass the check" $ \path ->
property $ \b s ->
not (B.null b) ==> do
let headerLength = 30 + (B.length . T.encodeUtf8 . getEntryName $ s)
let headerLength = 50 + (B.length . T.encodeUtf8 . getEntryName $ s)
localFileHeaderOffset <- createArchive path $ do
addEntry Store b s
commit
Expand Down

0 comments on commit da5df36

Please sign in to comment.