From 27da50f9d157927ec56dae8316d0edc34eaa244d Mon Sep 17 00:00:00 2001 From: Tolmachev Igor Date: Tue, 27 Aug 2024 18:03:30 +0900 Subject: Rewrite Eocdr64Locator search algorithm --- src/zip/driver.rs | 70 ++++++++++++++++++++++++----------------------- src/zip/error.rs | 8 +++--- tests/zip.rs | 81 +++++++++++++++++++++++++++++++++++-------------------- 3 files changed, 93 insertions(+), 66 deletions(-) diff --git a/src/zip/driver.rs b/src/zip/driver.rs index 7758479..0502c85 100644 --- a/src/zip/driver.rs +++ b/src/zip/driver.rs @@ -79,7 +79,7 @@ impl ArchiveRead for Zip { // Search eocdr let limit = 65557.min(io.seek(SeekFrom::End(0))?) as i64; let start = io.seek(SeekFrom::End(-limit))?; - let pos = start + let eocdr_pos = start + io.read_vec( (limit as usize) .checked_sub(18) @@ -90,46 +90,50 @@ impl ArchiveRead for Zip { .ok_or(ZipError::StructNotFound("Eocdr"))? as u64; // Read eocdr - io.seek(SeekFrom::Start(pos + 4))?; + io.seek(SeekFrom::Start(eocdr_pos + 4))?; let buf = io.read_arr::<18>()?; let eocdr: Eocdr = deserialize(&buf).unwrap(); let comment = String::from_cp437(io.read_vec(eocdr.comment_len as usize)?); + let mut cd_pointer = eocdr.cd_pointer as u64; + let mut cd_size = eocdr.cd_size as u64; + let mut cd_records = eocdr.cd_records as u64; + // Try to find eocdr64locator - io.seek(SeekFrom::Start(pos.saturating_sub(20)))?; - let buf = io.read_arr::<20>()?; - let (cd_pointer, cd_size, cd_records) = if buf[..4] == EOCDR64_LOCATOR_SIGNATURE { - // Locator found - let eocdr64locator: Eocdr64Locator = deserialize(&buf[4..]).unwrap(); - - io.seek(SeekFrom::Start(eocdr64locator.eocdr64_pointer))?; - if io.read_arr()? != EOCDR64_SIGNATURE { - return Err(ZipError::InvalidSignature("Eocdr64")); - } + if eocdr_pos >= 20 { + io.seek(SeekFrom::Start(eocdr_pos - 20))?; + let buf = io.read_arr::<20>()?; - let eocdr64: Eocdr64 = deserialize(&io.read_arr::<52>()?).unwrap(); - if eocdr64.cd_pointer + eocdr64.cd_size > eocdr64locator.eocdr64_pointer { - return Err(ZipError::Overlapping( - "Central directory records", - "Zip64 end of central directory record", - )); - } + if buf[..4] == EOCDR64_LOCATOR_SIGNATURE { + let locator: Eocdr64Locator = deserialize(&buf[4..]).unwrap(); + io.seek(SeekFrom::Start(locator.eocdr64_pointer))?; - (eocdr64.cd_pointer, eocdr64.cd_size, eocdr64.cd_records) - } else { - if (eocdr.cd_pointer + eocdr.cd_size) as u64 > pos { - return Err(ZipError::Overlapping( - "Central directory records", - "End of central directory record", - )); - } + if io.read_arr()? != EOCDR64_SIGNATURE { + return Err(ZipError::InvalidSignature("Eocdr64")); + } + + if locator.eocdr64_pointer + 76 > eocdr_pos { + return Err(ZipError::Overlapping("Eocdr64", "Eocdr64Locator")); + } - ( - eocdr.cd_pointer as u64, - eocdr.cd_size as u64, - eocdr.cd_records as u64, - ) - }; + let eocdr64: Eocdr64 = deserialize(&io.read_arr::<54>()?).unwrap(); + if locator.eocdr64_pointer + eocdr64.eocdr64_size + 32 > eocdr_pos { + return Err(ZipError::Overlapping("Eocdr64", "Eocdr64Locator")); + } + + cd_pointer = eocdr64.cd_pointer; + cd_size = eocdr64.cd_size; + cd_records = eocdr64.cd_records; + + if cd_pointer + cd_size > locator.eocdr64_pointer { + return Err(ZipError::Overlapping("Cdr", "Eocdr64")); + } + } else if cd_pointer + cd_size > eocdr_pos { + return Err(ZipError::Overlapping("Cdr", "Eocdr")); + } + } else if cd_pointer + cd_size > eocdr_pos { + return Err(ZipError::Overlapping("Cdr", "Eocdr")); + } // Read cd records let mut indexes = Map::with_capacity(cd_records as usize); diff --git a/src/zip/error.rs b/src/zip/error.rs index fbc2ba1..40d45fe 100644 --- a/src/zip/error.rs +++ b/src/zip/error.rs @@ -31,10 +31,10 @@ impl From for ZipError { impl PartialEq for ZipError { fn eq(&self, other: &Self) -> bool { match (self, other) { - (Self::Io(l0), Self::Io(r0)) => l0.kind() == r0.kind(), - (Self::StructNotFound(l0), Self::StructNotFound(r0)) => l0 == r0, - (Self::InvalidSignature(l0), Self::InvalidSignature(r0)) => l0 == r0, - (Self::InvalidField(l0), Self::InvalidField(r0)) => l0 == r0, + (Self::Io(l), Self::Io(r)) => l.kind() == r.kind(), + (Self::StructNotFound(l), Self::StructNotFound(r)) => l == r, + (Self::InvalidSignature(l), Self::InvalidSignature(r)) => l == r, + (Self::InvalidField(l), Self::InvalidField(r)) => l == r, (Self::Overlapping(l0, l1), Self::Overlapping(r0, r1)) => l0 == r0 && l1 == r1, _ => core::mem::discriminant(self) == core::mem::discriminant(other), } diff --git a/tests/zip.rs b/tests/zip.rs index 1422bb3..e00789d 100644 --- a/tests/zip.rs +++ b/tests/zip.rs @@ -90,11 +90,17 @@ fn test_zip_weak() { } } -const EMPTY: Cursor<&[u8]> = Cursor::new(b"PK\x05\x06\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"); - #[test] fn test_zip() { - assert_eq!(Archive::>::read(EMPTY).unwrap().len(), 0); + assert_eq!( + Archive::>::read(Cursor::new(&[ + 0x50, 0x4b, 0x05, 0x06, 0, 0, 0, 0, 0, // + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // Eocdr + ])) + .unwrap() + .len(), + 0 + ); let mut archive = Archive::::read_from_file("tests/files/archive.zip").unwrap(); @@ -158,32 +164,49 @@ fn test_zip() { } } -const NOT_FOUND: Cursor<&[u8]> = Cursor::new(b""); -const INVALID: Cursor<&[u8]> = Cursor::new( - b"PK\x06\x07\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0PK\x05\x06\0\0\0\0\0\0\0\0\x01\0\0\0\0\0\0\0\0\0", -); -const OVERLAP: Cursor<&[u8]> = Cursor::new(b"PK\x05\x06\0\0\0\0\0\0\0\0\x01\0\0\0\0\0\0\0\0\0"); -const OVERLAP64: Cursor<&[u8]> = Cursor::new(b"PK\x06\x06\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\x01\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0PK\x06\x07\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0PK\x05\x06\0\0\0\0\0\0\0\0\x01\0\0\0\0\0\0\0\0\0"); - #[test] fn test_bad_zip() { - assert!( - Archive::>::read(NOT_FOUND).is_err_and(|e| e == ZipError::StructNotFound("Eocdr")) - ); - - assert!( - Archive::>::read(INVALID).is_err_and(|e| e == ZipError::InvalidSignature("Eocdr64")) - ); - - assert!(Archive::>::read(OVERLAP).is_err_and(|e| e - == ZipError::Overlapping( - "Central directory records", - "End of central directory record" - ))); - - assert!(Archive::>::read(OVERLAP64).is_err_and(|e| e - == ZipError::Overlapping( - "Central directory records", - "Zip64 end of central directory record" - ))); + assert!(Archive::>::read(Cursor::new(&[])) + .is_err_and(|e| e == ZipError::StructNotFound("Eocdr"))); + + assert!(Archive::>::read(Cursor::new(&[ + // No Eocdr64 + 0x50, 0x4b, 0x06, 0x07, 0, 0, 0, 0, // + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // Eocdr64Locator + // + 0x50, 0x4b, 0x05, 0x06, 0, 0, 0, 0, 0, // + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // Eocdr + ])) + .is_err_and(|e| e == ZipError::InvalidSignature("Eocdr64"))); + + assert!(Archive::>::read(Cursor::new(&[ + 0x50, 0x4b, 0x06, 0x06, // Eocdr64 + // + 0x50, 0x4b, 0x06, 0x07, 0, 0, 0, 0, // + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // Eocdr64Locator + // + 0x50, 0x4b, 0x05, 0x06, 0, 0, 0, 0, 0, // + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // Eocdr + ])) + .is_err_and(|e| e == ZipError::Overlapping("Eocdr64", "Eocdr64Locator"))); + + assert!(Archive::>::read(Cursor::new(&[ + // No records + 0x50, 0x4b, 0x05, 0x06, 0, 0, 0, 0, 0, // + 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, // Eocdr + ])) + .is_err_and(|e| e == ZipError::Overlapping("Cdr", "Eocdr"))); + + assert!(Archive::>::read(Cursor::new(&[ + 0x50, 0x4b, 0x06, 0x06, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, // Eocdr64 + // + 0x50, 0x4b, 0x06, 0x07, 0, 0, 0, 0, // + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // Eocdr64Locator + // + 0x50, 0x4b, 0x05, 0x06, 0, 0, 0, 0, 0, // + 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, // Eocdr + ])) + .is_err_and(|e| e == ZipError::Overlapping("Cdr", "Eocdr64"))); } -- cgit v1.2.3