Incorrectly created ZIP_EM_TRAD_PKWARE-encrypted entries from sources which have no mtime
QrczakMK opened this issue · 2 comments
A ZIP_EM_TRAD_PKWARE-encrypted entry created from a zip_source_t whose ZIP_SOURCE_STAT does not set ZIP_STAT_MTIME, with st.mtime left as -1, results in an archive which fails password verification.
This is because here: https://github.com/nih-at/libzip/blob/main/lib/zip_source_pkware_encode.c#L93 the st.mtime of -1 is converted to dostime and its higher byte is used in the random header for encryption.
Then here: https://github.com/nih-at/libzip/blob/main/lib/zip_close.c#L530 when the directory entry is written, the absent st.mtime is assumed to be the current time instead of converting the -1.
Finally here: https://github.com/nih-at/libzip/blob/main/lib/zip_source_pkware_decode.c#L111 the stored modification time is converted to dostime and its higher byte is compared with the byte stored in the header for encryption, which usually fails.
Since ZIP_SOURCE_STAT is called separately in both places, this may also produce an invalid archive when ZIP_SOURCE_STAT returns different modification times, e.g. if someone changes the modification time of a real file concurrently with archive creation. This is perhaps less important in practice but still worrying — it is understood that archiving a file concurrently with modifying it might produce weird results, but it is unexpected that it can fail password verification.
BTW, implicitly using the current time for modification times has a minor weird behavior that different entries may get slightly different times, reflecting the time passed between creating these entries. Maybe it would be nicer to get a consistent snapshot of the current time when the archive is created. The same snapshot could then be used for encryption.
We can't reproduce the problem, but from reading the source it's clear that that case was not handled correctly.
We have committed a fix, could you please verify that it works for you?