Skip to content

Commit

Permalink
fix: Could still select a fake CDE over a real one in some cases
Browse files Browse the repository at this point in the history
  • Loading branch information
Pr0methean committed Jun 19, 2024
1 parent 4c2e9f6 commit 78a38e9
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 1 deletion.
6 changes: 5 additions & 1 deletion src/read.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ pub(crate) mod zip_archive {
pub(crate) files: Vec<super::ZipFileData>,
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,
Expand Down Expand Up @@ -721,7 +722,7 @@ impl<R: Read + Seek> ZipArchive<R> {
}
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()))
Expand All @@ -748,10 +749,12 @@ impl<R: Read + Seek> ZipArchive<R> {
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,
})
}
Expand Down Expand Up @@ -820,6 +823,7 @@ impl<R: Read + Seek> ZipArchive<R> {
///
/// 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<ZipArchive<R>> {
reader.seek(SeekFrom::Start(0))?;
if let Ok((footer, shared)) = Self::get_metadata(config, &mut reader) {
return Ok(ZipArchive {
reader,
Expand Down
43 changes: 43 additions & 0 deletions src/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -625,6 +625,7 @@ impl<A: Read + Write + Seek> ZipWriter<A> {
///
/// This uses the given read configuration to initially read the archive.
pub fn new_append_with_config(config: Config, mut readwriter: A) -> ZipResult<ZipWriter<A>> {
readwriter.seek(SeekFrom::Start(0))?;
if let Ok((footer, shared)) = ZipArchive::get_metadata(config, &mut readwriter) {
Ok(ZipWriter {
inner: Storer(MaybeEncrypted::Unencrypted(readwriter)),
Expand Down Expand Up @@ -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(())
}
}

0 comments on commit 78a38e9

Please sign in to comment.