Skip to content

salt the pkware encryption header per entry in _zip_entry_open#444

Merged
kuba-- merged 2 commits into
kuba--:masterfrom
jmestwa-coder:pkware-header-per-entry-salt
Jul 18, 2026
Merged

salt the pkware encryption header per entry in _zip_entry_open#444
kuba-- merged 2 commits into
kuba--:masterfrom
jmestwa-coder:pkware-header-per-entry-salt

Conversation

@jmestwa-coder

Copy link
Copy Markdown
Contributor

encryption header is identical for every entry under the same password
The header's first 11 bytes came from a crc of the bytes written so far, which depends only on the password, so identical plaintext encrypted to identical ciphertext and entries reused the keystream. Salted the derivation with the entry's own local-header offset and time so each entry gets a distinct header; encrypt and decrypt stay symmetric so existing archives still read back.

Comment thread src/zip.c Outdated
(mz_uint8)((mz_crc32(MZ_CRC32_INIT, enc_header, i) ^ salt) >>
(i & 7));
enc_header[i] = zip_pkware_encrypt_byte(&zip->entry.enc_keys, rnd);
salt = salt * 1103515245u + 12345u;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Comment + #define naming the LCG.
  • Simpler mix:
salt = (salt << 5) ^ (salt >> 7) ^ (mz_uint32)i 

— also fine for uniqueness.
True random for the 11 bytes — better hygiene, more platform work.

  • (Optionally): True random for the 11 bytes — better hygiene, more platform work.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

switched to your shift/xor mix, so the lcg constants are gone and the comment now describes the stir instead. left true random out for now to keep it portable, happy to follow up with it if you want. full suite still passes, including the determinism test.

@kuba--
kuba-- merged commit f6fc280 into kuba--:master Jul 18, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants