From ff4dee28d78127dc3b28f9e75b7787bacc1936f8 Mon Sep 17 00:00:00 2001 From: Chris Hennick Date: Sat, 13 May 2023 15:38:09 -0700 Subject: [PATCH] Bug fix --- fuzz/fuzz_targets/fuzz_write.rs | 32 +++++++++++++++++++++++++++----- src/types.rs | 15 ++++++++++++++- src/write.rs | 30 +++++++++++++++++++----------- 3 files changed, 60 insertions(+), 17 deletions(-) diff --git a/fuzz/fuzz_targets/fuzz_write.rs b/fuzz/fuzz_targets/fuzz_write.rs index ba8da3cf..261c7601 100644 --- a/fuzz/fuzz_targets/fuzz_write.rs +++ b/fuzz/fuzz_targets/fuzz_write.rs @@ -4,6 +4,7 @@ use std::cell::RefCell; use libfuzzer_sys::fuzz_target; use arbitrary::Arbitrary; use std::io::{Cursor, Read, Seek, Write}; +use std::path::{PathBuf}; #[derive(Arbitrary,Debug)] pub struct File { @@ -13,11 +14,22 @@ pub struct File { #[derive(Arbitrary,Debug)] pub enum FileOperation { - Write { + WriteNormalFile { file: File, options: zip_next::write::FileOptions, reopen: bool, }, + WriteDirectory { + name: String, + options: zip_next::write::FileOptions, + reopen: bool, + }, + WriteSymlink { + name: String, + target: Box, + options: zip_next::write::FileOptions, + reopen: bool, + }, ShallowCopy { base: Box, new_name: String, @@ -33,7 +45,9 @@ pub enum FileOperation { impl FileOperation { pub fn get_name(&self) -> String { match self { - FileOperation::Write {file, ..} => &file.name, + FileOperation::WriteNormalFile {file, ..} => &file.name, + FileOperation::WriteDirectory {name, ..} => name, + FileOperation::WriteSymlink {name, ..} => name, FileOperation::ShallowCopy {new_name, ..} => new_name, FileOperation::DeepCopy {new_name, ..} => new_name }.to_owned() @@ -41,9 +55,11 @@ impl FileOperation { pub fn should_reopen(&self) -> bool { match self { - FileOperation::Write {reopen, ..} => *reopen, + FileOperation::WriteNormalFile {reopen, ..} => *reopen, FileOperation::ShallowCopy {reopen, ..} => *reopen, - FileOperation::DeepCopy {reopen, ..} => *reopen + FileOperation::DeepCopy {reopen, ..} => *reopen, + FileOperation::WriteDirectory {reopen, ..} => *reopen, + FileOperation::WriteSymlink {reopen, ..} => *reopen, } } } @@ -53,7 +69,7 @@ fn do_operation(writer: &mut RefCell>, where T: Read + Write + Seek { let should_reopen = operation.should_reopen(); match operation { - FileOperation::Write {file, mut options, ..} => { + FileOperation::WriteNormalFile {file, mut options, ..} => { if file.contents.iter().map(Vec::len).sum::() >= u32::MAX as usize { options = options.large_file(true); } @@ -62,6 +78,12 @@ fn do_operation(writer: &mut RefCell>, writer.borrow_mut().write_all(chunk.as_slice())?; } } + FileOperation::WriteDirectory {name, options, ..} => { + writer.borrow_mut().add_directory(name, options)?; + } + FileOperation::WriteSymlink {name, target, options, ..} => { + writer.borrow_mut().add_symlink(name, target.to_string_lossy(), options)?; + } FileOperation::ShallowCopy {base, ref new_name, .. } => { let base_name = base.get_name(); do_operation(writer, *base)?; diff --git a/src/types.rs b/src/types.rs index 52c0b6ea..d6d1f14f 100644 --- a/src/types.rs +++ b/src/types.rs @@ -92,7 +92,6 @@ impl System { /// Modern zip files store more precise timestamps, which are ignored by [`crate::read::ZipArchive`], /// so keep in mind that these timestamps are unreliable. [We're working on this](https://github.com/zip-rs/zip/issues/156#issuecomment-652981904). #[derive(Debug, Clone, Copy)] -#[cfg_attr(fuzzing, derive(arbitrary::Arbitrary))] pub struct DateTime { year: u16, month: u8, @@ -102,6 +101,20 @@ pub struct DateTime { second: u8, } +#[cfg(fuzzing)] +impl arbitrary::Arbitrary<'_> for DateTime { + fn arbitrary(u: &mut arbitrary::Unstructured) -> arbitrary::Result { + Ok(DateTime { + year: u.int_in_range(1980..=2107)?, + month: u.int_in_range(1..=12)?, + day: u.int_in_range(1..=31)?, + hour: u.int_in_range(0..=23)?, + minute: u.int_in_range(0..=59)?, + second: u.int_in_range(0..=60)?, + }) + } +} + impl Default for DateTime { /// Constructs an 'default' datetime of 1980-01-01 00:00:00 fn default() -> DateTime { diff --git a/src/write.rs b/src/write.rs index 2dc7fe37..1bdfb6be 100644 --- a/src/write.rs +++ b/src/write.rs @@ -646,14 +646,7 @@ impl ZipWriter { .inner .prepare_next_writer(CompressionMethod::Stored, None)?; self.inner.switch_to(make_plain_writer)?; - match mem::replace(&mut self.inner, Closed) { - Storer(MaybeEncrypted::Encrypted(writer)) => { - let crc32 = self.stats.hasher.clone().finalize(); - self.inner = Storer(MaybeEncrypted::Unencrypted(writer.finish(crc32)?)) - } - Storer(w) => self.inner = Storer(w), - _ => unreachable!(), - } + self.switch_to_non_encrypting_writer()?; let writer = self.inner.get_plain(); if !self.writing_raw { @@ -676,18 +669,32 @@ impl ZipWriter { Ok(()) } + fn switch_to_non_encrypting_writer(&mut self) -> Result<(), ZipError> { + match mem::replace(&mut self.inner, Closed) { + Storer(MaybeEncrypted::Encrypted(writer)) => { + let crc32 = self.stats.hasher.clone().finalize(); + self.inner = Storer(MaybeEncrypted::Unencrypted(writer.finish(crc32)?)) + } + Storer(MaybeEncrypted::Unencrypted(w)) => { + self.inner = Storer(MaybeEncrypted::Unencrypted(w)) + } + _ => unreachable!(), + } + Ok(()) + } + /// Removes the file currently being written from the archive if there is one, or else removes /// the file most recently written. pub fn abort_file(&mut self) -> ZipResult<()> { let last_file = self.files.pop().ok_or(ZipError::FileNotFound)?; self.files_by_name.remove(&last_file.file_name); - self.inner - .get_plain() - .seek(SeekFrom::Start(last_file.header_start))?; let make_plain_writer = self .inner .prepare_next_writer(CompressionMethod::Stored, None)?; self.inner.switch_to(make_plain_writer)?; + self.inner + .get_plain() + .seek(SeekFrom::Start(last_file.header_start))?; self.writing_to_file = false; Ok(()) } @@ -843,6 +850,7 @@ impl ZipWriter { self.start_entry(name_with_slash, options, None)?; self.writing_to_file = false; + self.switch_to_non_encrypting_writer()?; Ok(()) }