From 64b5debc16cd3583b4bce0ea74ae0914004a8470 Mon Sep 17 00:00:00 2001 From: Chris Hennick <4961925+Pr0methean@users.noreply.github.com> Date: Mon, 17 Jun 2024 12:06:37 -0700 Subject: [PATCH] chore: Fix another fuzz failure --- fuzz/fuzz_targets/fuzz_write.rs | 22 +++++--- src/unstable.rs | 25 +++++---- src/write.rs | 96 ++++++++++++++++++++++++++++++++- src/zipcrypto.rs | 11 +++- 4 files changed, 133 insertions(+), 21 deletions(-) diff --git a/fuzz/fuzz_targets/fuzz_write.rs b/fuzz/fuzz_targets/fuzz_write.rs index 6cd44aa3..eb33217e 100755 --- a/fuzz/fuzz_targets/fuzz_write.rs +++ b/fuzz/fuzz_targets/fuzz_write.rs @@ -42,6 +42,16 @@ pub struct FileOperation<'k> { // 'abort' flag is separate, to prevent trying to copy an aborted file } +impl <'k> FileOperation<'k> { + fn get_path(&self) -> Cow { + if let BasicFileOperation::WriteDirectory(_) = self.basic { + Cow::Owned(self.path.join("/")) + } else { + Cow::Borrowed(&self.path) + } + } +} + impl <'k> Debug for FileOperation<'k> { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match &self.basic { @@ -63,10 +73,10 @@ impl <'k> Debug for FileOperation<'k> { options, self.path, target.to_owned()))?; }, BasicFileOperation::ShallowCopy(base) => { - f.write_fmt(format_args!("{:?}writer.shallow_copy_file_from_path({:?}, {:?})?;\n", base, base.path, self.path))?; + f.write_fmt(format_args!("{:?}writer.shallow_copy_file_from_path({:?}, {:?})?;\n", base, base.get_path(), self.path))?; }, BasicFileOperation::DeepCopy(base) => { - f.write_fmt(format_args!("{:?}writer.deep_copy_file_from_path({:?}, {:?})?;\n", base, base.path, self.path))?; + f.write_fmt(format_args!("{:?}writer.deep_copy_file_from_path({:?}, {:?})?;\n", base, base.get_path(), self.path))?; }, BasicFileOperation::MergeWithOtherFile {operations} => { f.write_str("let sub_writer = {\n\ @@ -177,15 +187,15 @@ where *files_added += 1; } BasicFileOperation::ShallowCopy(base) => { - deduplicate_paths(&mut path, &base.path); + deduplicate_paths(&mut path, &base.get_path()); do_operation(writer, &base, false, flush_on_finish_file, files_added)?; - writer.shallow_copy_file_from_path(&base.path, &*path)?; + writer.shallow_copy_file_from_path(&*base.get_path(), &*path)?; *files_added += 1; } BasicFileOperation::DeepCopy(base) => { - deduplicate_paths(&mut path, &base.path); + deduplicate_paths(&mut path, &base.get_path()); do_operation(writer, &base, false, flush_on_finish_file, files_added)?; - writer.deep_copy_file_from_path(&base.path, &*path)?; + writer.deep_copy_file_from_path(&*base.get_path(), &*path)?; *files_added += 1; } BasicFileOperation::MergeWithOtherFile { operations } => { diff --git a/src/unstable.rs b/src/unstable.rs index 7584f7f6..f7130dad 100644 --- a/src/unstable.rs +++ b/src/unstable.rs @@ -74,22 +74,21 @@ impl LittleEndianReadExt for R {} pub fn path_to_string>(path: T) -> Box { let mut maybe_original = None; if let Some(original) = path.as_ref().to_str() { - if original.is_empty() { + if original.is_empty() || original == "." || original == ".." { return String::new().into_boxed_str(); } - if (MAIN_SEPARATOR == '/' || !original[1..].contains(MAIN_SEPARATOR)) - && !original.ends_with('.') - && !original.starts_with(['.', MAIN_SEPARATOR]) - && !original.starts_with(['.', '.', MAIN_SEPARATOR]) - && !original.contains([MAIN_SEPARATOR, MAIN_SEPARATOR]) - && !original.contains([MAIN_SEPARATOR, '.', MAIN_SEPARATOR]) - && !original.contains([MAIN_SEPARATOR, '.', '.', MAIN_SEPARATOR]) - { - if original.starts_with(MAIN_SEPARATOR) { + if original.starts_with(MAIN_SEPARATOR) { + if original.len() == 1 { + return MAIN_SEPARATOR.to_string().into_boxed_str(); + } else if (MAIN_SEPARATOR == '/' || !original[1..].contains(MAIN_SEPARATOR)) + && !original.ends_with('.') + && !original.contains([MAIN_SEPARATOR, MAIN_SEPARATOR]) + && !original.contains([MAIN_SEPARATOR, '.', MAIN_SEPARATOR]) + && !original.contains([MAIN_SEPARATOR, '.', '.', MAIN_SEPARATOR]) { maybe_original = Some(&original[1..]); - } else { - maybe_original = Some(original); } + } else if !original.contains(MAIN_SEPARATOR) { + return original.into(); } } let mut recreate = maybe_original.is_none(); @@ -107,7 +106,7 @@ pub fn path_to_string>(path: T) -> Box { Component::ParentDir => { recreate = true; normalized_components.pop(); - } + }, _ => { recreate = true; } diff --git a/src/write.rs b/src/write.rs index 4054a373..d74e506a 100644 --- a/src/write.rs +++ b/src/write.rs @@ -754,7 +754,9 @@ impl ZipWriter { src_path: T, dest_path: U, ) -> ZipResult<()> { - self.deep_copy_file(&path_to_string(src_path), &path_to_string(dest_path)) + let src = path_to_string(src_path); + let dest = path_to_string(dest_path); + self.deep_copy_file(&src, &dest) } /// Write the zip file into the backing stream, then produce a readable archive of that data. @@ -1967,7 +1969,10 @@ mod test { use crate::ZipArchive; use std::io; use std::io::{Cursor, Read, Write}; + use std::marker::PhantomData; use std::path::PathBuf; + use crate::write::EncryptWith::ZipCrypto; + use crate::zipcrypto::ZipCryptoKeys; #[test] fn write_empty_zip() { @@ -2900,4 +2905,93 @@ mod test { let _ = writer.finish_into_readable()?; Ok(()) } + + #[allow(deprecated)] + #[test] + fn test_fuzz_crash_2024_06_17() -> ZipResult<()> { + let mut writer = ZipWriter::new(Cursor::new(Vec::new())); + writer.set_flush_on_finish_file(false); + let sub_writer = { + let mut writer = ZipWriter::new(Cursor::new(Vec::new())); + writer.set_flush_on_finish_file(false); + let sub_writer = { + let mut writer = ZipWriter::new(Cursor::new(Vec::new())); + writer.set_flush_on_finish_file(false); + let sub_writer = { + let mut writer = ZipWriter::new(Cursor::new(Vec::new())); + writer.set_flush_on_finish_file(false); + let sub_writer = { + let mut writer = ZipWriter::new(Cursor::new(Vec::new())); + writer.set_flush_on_finish_file(false); + let sub_writer = { + let mut writer = ZipWriter::new(Cursor::new(Vec::new())); + writer.set_flush_on_finish_file(false); + let sub_writer = { + let mut writer = ZipWriter::new(Cursor::new(Vec::new())); + writer.set_flush_on_finish_file(false); + let sub_writer = { + let mut writer = ZipWriter::new(Cursor::new(Vec::new())); + writer.set_flush_on_finish_file(false); + let sub_writer = { + let mut writer = ZipWriter::new(Cursor::new(Vec::new())); + writer.set_flush_on_finish_file(false); + let sub_writer = { + let mut writer = ZipWriter::new(Cursor::new(Vec::new())); + writer.set_flush_on_finish_file(false); + let options = FileOptions { compression_method: CompressionMethod::Unsupported(65535), compression_level: Some(5), last_modified_time: DateTime::from_date_and_time(2107, 2, 8, 15, 0, 0)?, permissions: None, large_file: true, encrypt_with: Some(ZipCrypto(ZipCryptoKeys::of(0x63ff,0xc62d3103,0xfffe00ea), PhantomData::default())), extended_options: ExtendedFileOptions {extra_data: vec![].into(), central_extra_data: vec![].into()}, alignment: 255, ..Default::default() }; + writer.add_symlink_from_path("1\0PK\u{6}\u{6}\u{b}\u{6}\u{6}\u{6}\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\u{1}\0\0\0\0\0\0\0\0\u{b}\0\0PK\u{1}\u{2},\0\0\0\0\0\0\0\0\0\0\0\u{10}\0\0\0K\u{6}\u{6}\0\0\0\0\0\0\0\0PK\u{2}\u{6}", "", options)?; + writer = ZipWriter::new_append(writer.finish_into_readable()?.into_inner())?; + writer + }; + writer.merge_archive(sub_writer.finish_into_readable()?)?; + writer = ZipWriter::new_append(writer.finish_into_readable()?.into_inner())?; + let options = FileOptions { compression_method: Stored, compression_level: None, last_modified_time: DateTime::from_date_and_time(1992, 7, 3, 0, 0, 0)?, permissions: None, large_file: true, encrypt_with: None, extended_options: ExtendedFileOptions {extra_data: vec![].into(), central_extra_data: vec![].into()}, alignment: 43, ..Default::default() }; + writer.start_file_from_path("\0\0\0\u{3}\0\u{1a}\u{1a}\u{1a}\u{1a}\u{1a}\u{1a}", options)?; + let options = FileOptions { compression_method: Stored, compression_level: None, last_modified_time: DateTime::from_date_and_time(2006, 3, 27, 2, 24, 26)?, permissions: None, large_file: false, encrypt_with: None, extended_options: ExtendedFileOptions {extra_data: vec![].into(), central_extra_data: vec![].into()}, alignment: 26, ..Default::default() }; + writer.start_file_from_path("\0K\u{6}\u{6}\0PK\u{6}\u{7}PK\u{6}\u{6}\0\0\0\0\0\0\0\0PK\u{2}\u{6}", options)?; + writer = ZipWriter::new_append(writer.finish_into_readable()?.into_inner())?; + let options = FileOptions { compression_method: Stored, compression_level: Some(17), last_modified_time: DateTime::from_date_and_time(2103, 4, 10, 23, 15, 18)?, permissions: Some(3284386755), large_file: true, encrypt_with: Some(ZipCrypto(ZipCryptoKeys::of(0x8888c5bf,0x88888888,0xff888888), PhantomData::default())), extended_options: ExtendedFileOptions {extra_data: vec![3, 0, 1, 0, 255, 144, 136, 0, 0].into(), central_extra_data: vec![].into()}, alignment: 65535, ..Default::default() }; + writer.add_symlink_from_path("", "\nu", options)?; + writer = ZipWriter::new_append(writer.finish()?)?; + writer + }; + writer.merge_archive(sub_writer.finish_into_readable()?)?; + writer = ZipWriter::new_append(writer.finish_into_readable()?.into_inner())?; + writer + }; + writer.merge_archive(sub_writer.finish_into_readable()?)?; + writer = ZipWriter::new_append(writer.finish()?)?; + writer + }; + writer.merge_archive(sub_writer.finish_into_readable()?)?; + writer = ZipWriter::new_append(writer.finish_into_readable()?.into_inner())?; + writer.abort_file()?; + let options = FileOptions { compression_method: CompressionMethod::Unsupported(49603), compression_level: Some(20), last_modified_time: DateTime::from_date_and_time(2047, 4, 14, 3, 15, 14)?, permissions: Some(3284386755), large_file: true, encrypt_with: Some(ZipCrypto(ZipCryptoKeys::of( 0xc3, 0x0, 0x0), PhantomData::default())), extended_options: ExtendedFileOptions {extra_data: vec![].into(), central_extra_data: vec![].into()}, alignment: 0, ..Default::default() }; + writer.add_directory_from_path("", options)?; + writer.deep_copy_file_from_path("/", "")?; + writer.shallow_copy_file_from_path("", "copy")?; + assert!(writer.shallow_copy_file_from_path("", "copy").is_err()); + assert!(writer.shallow_copy_file_from_path("", "copy").is_err()); + assert!(writer.shallow_copy_file_from_path("", "copy").is_err()); + assert!(writer.shallow_copy_file_from_path("", "copy").is_err()); + assert!(writer.shallow_copy_file_from_path("", "copy").is_err()); + assert!(writer.shallow_copy_file_from_path("", "copy").is_err()); + writer + }; + writer.merge_archive(sub_writer.finish_into_readable()?)?; + writer + }; + writer.merge_archive(sub_writer.finish_into_readable()?)?; + writer + }; + writer.merge_archive(sub_writer.finish_into_readable()?)?; + writer + }; + writer.merge_archive(sub_writer.finish_into_readable()?)?; + writer + }; + writer.merge_archive(sub_writer.finish_into_readable()?)?; + let _ = writer.finish_into_readable()?; + Ok(()) + } } diff --git a/src/zipcrypto.rs b/src/zipcrypto.rs index 7baac54b..bd81b5c4 100644 --- a/src/zipcrypto.rs +++ b/src/zipcrypto.rs @@ -31,7 +31,7 @@ impl Debug for ZipCryptoKeys { } #[cfg(any(test, fuzzing))] return f.write_fmt(format_args!( - "ZipCryptoKeys({:#10x},{:#10x},{:#10x})", + "ZipCryptoKeys::of({:#10x},{:#10x},{:#10x})", self.key_0, self.key_1, self.key_2 )); } @@ -46,6 +46,15 @@ impl ZipCryptoKeys { } } + #[allow(unused)] + pub const fn of(key_0: u32, key_1: u32, key_2: u32) -> ZipCryptoKeys { + ZipCryptoKeys { + key_0: Wrapping(key_0), + key_1: Wrapping(key_1), + key_2: Wrapping(key_2), + } + } + fn update(&mut self, input: u8) { self.key_0 = ZipCryptoKeys::crc32(self.key_0, input); self.key_1 =