From 78a38e977af3519db5ab5a74666b2fb57134788e Mon Sep 17 00:00:00 2001 From: Chris Hennick <4961925+Pr0methean@users.noreply.github.com> Date: Tue, 18 Jun 2024 22:33:00 -0700 Subject: [PATCH] fix: Could still select a fake CDE over a real one in some cases --- src/read.rs | 6 +++++- src/write.rs | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/src/read.rs b/src/read.rs index 0cf371a3..9479b6a4 100644 --- a/src/read.rs +++ b/src/read.rs @@ -69,6 +69,7 @@ pub(crate) mod zip_archive { pub(crate) files: Vec, pub(super) offset: u64, pub(super) dir_start: u64, + pub(super) dir_end: u64, // This isn't yet used anywhere, but it is here for use cases in the future. #[allow(dead_code)] pub(super) config: super::Config, @@ -721,7 +722,7 @@ impl ZipArchive { } let (footer, shared) = ok_results .into_iter() - .max_by_key(|(_, shared)| (shared.dir_start - shared.offset, shared.dir_start)) + .max_by_key(|(_, shared)| (shared.dir_end, u64::MAX - shared.dir_start)) .unwrap(); reader.seek(io::SeekFrom::Start(shared.dir_start))?; Ok((Rc::try_unwrap(footer).unwrap(), shared.build())) @@ -748,10 +749,12 @@ impl ZipArchive { let file = central_header_to_zip_file(reader, dir_info.archive_offset)?; files.push(file); } + let dir_end = reader.stream_position()?; Ok(SharedBuilder { files, offset: dir_info.archive_offset, dir_start: dir_info.directory_start, + dir_end, config, }) } @@ -820,6 +823,7 @@ impl ZipArchive { /// /// This uses the central directory record of the ZIP file, and ignores local file headers. pub fn with_config(config: Config, mut reader: R) -> ZipResult> { + reader.seek(SeekFrom::Start(0))?; if let Ok((footer, shared)) = Self::get_metadata(config, &mut reader) { return Ok(ZipArchive { reader, diff --git a/src/write.rs b/src/write.rs index e4114f7d..982bbf36 100644 --- a/src/write.rs +++ b/src/write.rs @@ -625,6 +625,7 @@ impl ZipWriter { /// /// This uses the given read configuration to initially read the archive. pub fn new_append_with_config(config: Config, mut readwriter: A) -> ZipResult> { + readwriter.seek(SeekFrom::Start(0))?; if let Ok((footer, shared)) = ZipArchive::get_metadata(config, &mut readwriter) { Ok(ZipWriter { inner: Storer(MaybeEncrypted::Unencrypted(readwriter)), @@ -3368,4 +3369,46 @@ mod test { let _ = writer.finish_into_readable()?; Ok(()) } + + #[cfg(all(feature = "bzip2", feature = "aes-crypto"))] + #[test] + fn test_fuzz_crash_2024_06_18b() -> ZipResult<()> { + let mut writer = ZipWriter::new(Cursor::new(Vec::new())); + writer.set_flush_on_finish_file(true); + writer.set_raw_comment([0].into()); + writer = ZipWriter::new_append(writer.finish_into_readable()?.into_inner())?; + assert_eq!(writer.get_raw_comment()[0], 0); + let options = FileOptions { + compression_method: CompressionMethod::Bzip2, + compression_level: None, + last_modified_time: DateTime::from_date_and_time(2009, 6, 3, 13, 37, 39)?, + permissions: Some(2644352413), + large_file: true, + encrypt_with: Some(crate::write::EncryptWith::Aes { + mode: crate::AesMode::Aes256, + password: "", + }), + extended_options: ExtendedFileOptions { + extra_data: vec![].into(), + central_extra_data: vec![].into(), + }, + alignment: 255, + ..Default::default() + }; + writer.add_symlink_from_path("", "", options)?; + writer.deep_copy_file_from_path("", "PK\u{5}\u{6}\0\0\0\0\0\0\0\0\0\0\0\0\0\u{4}\0\0\0")?; + writer = ZipWriter::new_append(writer.finish_into_readable()?.into_inner())?; + assert_eq!(writer.get_raw_comment()[0], 0); + writer.deep_copy_file_from_path( + "PK\u{5}\u{6}\0\0\0\0\0\0\0\0\0\0\0\0\0\u{4}\0\0\0", + "\u{2}yy\u{5}qu\0", + )?; + let finished = writer.finish()?; + let archive = ZipArchive::new(finished.clone())?; + assert_eq!(archive.comment(), [0]); + writer = ZipWriter::new_append(finished)?; + assert_eq!(writer.get_raw_comment()[0], 0); + let _ = writer.finish_into_readable()?; + Ok(()) + } }