WIP: Write fuzzing

.github/workflows/ci.yaml

@@ -75,7 +75,7 @@ jobs:
     - name: Docs
       run: cargo doc
 
-  fuzz:
+  fuzz_read:
     runs-on: ubuntu-latest
 
     steps:
@@ -93,3 +93,22 @@ jobs:
       - name: run fuzz
         run: |
           cargo fuzz run fuzz_read -- -timeout=1s -runs=10000000
+
+  fuzz_write:
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions-rs/toolchain@v1
+        with:
+          profile: minimal
+          toolchain: nightly
+          override: true
+
+      - run: cargo install cargo-fuzz
+      - name: compile fuzz
+        run: |
+          cargo fuzz build fuzz_write
+      - name: run fuzz
+        run: |
+          cargo fuzz run fuzz_write -- -timeout=1s -runs=10000000

Cargo.toml

@@ -23,10 +23,14 @@ pbkdf2 = {version = "0.12.1", optional = true }
 sha1 = {version = "0.10.5", optional = true }
 time = { version = "0.3.20", optional = true, default-features = false, features = ["std"] }
 zstd = { version = "0.12.3", optional = true }
+replace_with = "0.1.7"
 
 [target.'cfg(any(all(target_arch = "arm", target_pointer_width = "32"), target_arch = "mips", target_arch = "powerpc"))'.dependencies]
 crossbeam-utils = "0.8.15"
 
+[target.'cfg(fuzzing)'.dependencies]
+arbitrary = { version = "1.3.0", features = ["derive"] }
+
 [dev-dependencies]
 bencher = "0.1.5"
 getrandom = "0.2.9"

fuzz/Cargo.toml

@@ -10,6 +10,7 @@ cargo-fuzz = true
 
 [dependencies]
 libfuzzer-sys = "0.4"
+arbitrary = { version = "1.3.0", features = ["derive"] }
 
 [dependencies.zip_next]
 path = ".."
@@ -23,3 +24,9 @@ name = "fuzz_read"
 path = "fuzz_targets/fuzz_read.rs"
 test = false
 doc = false
+
+[[bin]]
+name = "fuzz_write"
+path = "fuzz_targets/fuzz_write.rs"
+test = false
+doc = false

fuzz/fuzz_targets/fuzz_write.rs

@@ -0,0 +1,67 @@
+#![no_main]
+use libfuzzer_sys::fuzz_target;
+use arbitrary::Arbitrary;
+use std::io::{Cursor, Read, Seek, Write};
+
+#[derive(Arbitrary,Debug)]
+pub struct File {
+    pub name: String,
+    pub contents: Vec<u8>
+}
+
+#[derive(Arbitrary,Debug)]
+pub enum FileOperation {
+    Write {
+        file: File,
+        options: zip_next::write::FileOptions
+    },
+    ShallowCopy {
+        base: Box<FileOperation>,
+        new_name: String
+    },
+    DeepCopy {
+        base: Box<FileOperation>,
+        new_name: String
+    },
+}
+
+impl FileOperation {
+    pub fn get_name(&self) -> String {
+        match self {
+            FileOperation::Write {file, ..} => &file.name,
+            FileOperation::ShallowCopy {new_name, ..} => new_name,
+            FileOperation::DeepCopy {new_name, ..} => new_name
+        }.to_owned()
+    }
+}
+
+fn do_operation<T>(writer: &mut zip_next::ZipWriter<T>,
+                   operation: &FileOperation) -> Result<(), Box<dyn std::error::Error>>
+                   where T: Read + Write + Seek {
+    match operation {
+        FileOperation::Write {file, mut options} => {
+            if (*file).contents.len() >= u32::MAX as usize {
+                options = options.large_file(true);
+            }
+            writer.start_file(file.name.to_owned(), options)?;
+            writer.write_all(file.contents.as_slice())?;
+        }
+        FileOperation::ShallowCopy {base, new_name} => {
+            do_operation(writer, base)?;
+            writer.shallow_copy_file(&base.get_name(), new_name)?;
+        }
+        FileOperation::DeepCopy {base, new_name} => {
+            do_operation(writer, base)?;
+            writer.deep_copy_file(&base.get_name(), new_name)?;
+        }
+    }
+    Ok(())
+}
+
+fuzz_target!(|data: Vec<FileOperation>| {
+    let mut writer = zip_next::ZipWriter::new(Cursor::new(Vec::new()));
+    for operation in data {
+        let _ = do_operation(&mut writer, &operation);
+    }
+    writer.finish().unwrap();
+});

src/compression.rs

@@ -11,6 +11,7 @@ use std::fmt;
 /// When creating ZIP files, you may choose the method to use with
 /// [`crate::write::FileOptions::compression_method`]
 #[derive(Copy, Clone, PartialEq, Eq, Debug)]
+#[cfg_attr(fuzzing, derive(arbitrary::Arbitrary))]
 #[non_exhaustive]
 pub enum CompressionMethod {
     /// Store the file as is

src/types.rs

@@ -92,6 +92,7 @@ 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,
@@ -173,6 +174,11 @@ impl DateTime {
         }
     }
 
+    /// Indicates whether this date and time can be written to a zip archive.
+    pub fn is_valid(&self) -> bool {
+        DateTime::from_date_and_time(self.year, self.month, self.day, self.hour, self.minute, self.second).is_ok()
+    }
+
     #[cfg(feature = "time")]
     /// Converts a OffsetDateTime object to a DateTime
     ///

src/write.rs

@@ -24,6 +24,7 @@ use flate2::write::DeflateEncoder;
 
 #[cfg(feature = "bzip2")]
 use bzip2::write::BzEncoder;
+use replace_with::{replace_with, replace_with_and_return};
 
 #[cfg(feature = "time")]
 use time::OffsetDateTime;
@@ -91,6 +92,7 @@ pub(crate) mod zip_writer {
     }
 }
 pub use zip_writer::ZipWriter;
+use crate::write::GenericZipWriter::{Closed, Storer};
 
 #[derive(Default)]
 struct ZipWriterStats {
@@ -106,13 +108,14 @@ struct ZipRawValues {
 }
 
 /// Metadata for a file to be written
-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, Debug)]
+#[cfg_attr(fuzzing, derive(arbitrary::Arbitrary))]
 pub struct FileOptions {
-    compression_method: CompressionMethod,
-    compression_level: Option<i32>,
-    last_modified_time: DateTime,
-    permissions: Option<u32>,
-    large_file: bool,
+    pub(crate) compression_method: CompressionMethod,
+    pub(crate) compression_level: Option<i32>,
+    pub(crate) last_modified_time: DateTime,
+    pub(crate) permissions: Option<u32>,
+    pub(crate) large_file: bool,
 }
 
 impl FileOptions {
@@ -224,7 +227,8 @@ impl<W: Write + Seek> Write for ZipWriter<W> {
                         if self.stats.bytes_written > spec::ZIP64_BYTES_THR
                             && !self.files.last_mut().unwrap().large_file
                         {
-                            let _inner = mem::replace(&mut self.inner, GenericZipWriter::Closed);
+                            self.finish_file()?;
+                            self.files.pop();
                             return Err(io::Error::new(
                                 io::ErrorKind::Other,
                                 "Large file option has not been set",
@@ -236,7 +240,7 @@ impl<W: Write + Seek> Write for ZipWriter<W> {
             }
             None => Err(io::Error::new(
                 io::ErrorKind::BrokenPipe,
-                "ZipWriter was already closed",
+                "write(): ZipWriter was already closed",
             )),
         }
     }
@@ -246,7 +250,7 @@ impl<W: Write + Seek> Write for ZipWriter<W> {
             Some(ref mut w) => w.flush(),
             None => Err(io::Error::new(
                 io::ErrorKind::BrokenPipe,
-                "ZipWriter was already closed",
+                "flush(): ZipWriter was already closed",
             )),
         }
     }
@@ -504,6 +508,9 @@ impl<W: Write + Seek> ZipWriter<W> {
         if options.permissions.is_none() {
             options.permissions = Some(0o644);
         }
+        if !options.last_modified_time.is_valid() {
+            options.last_modified_time = FileOptions::default().last_modified_time;
+        }
         *options.permissions.as_mut().unwrap() |= ffi::S_IFREG;
     }
 
@@ -950,7 +957,11 @@ impl<W: Write + Seek> GenericZipWriter<W> {
     ) -> ZipResult<()> {
         match self.current_compression() {
             Some(method) if method == compression => return Ok(()),
-            None => {
+            _ => {}
+        }
+
+        match self {
+            GenericZipWriter::Closed => {
                 return Err(io::Error::new(
                     io::ErrorKind::BrokenPipe,
                     "ZipWriter was already closed",
@@ -959,20 +970,19 @@ impl<W: Write + Seek> GenericZipWriter<W> {
             }
             _ => {}
         }
-
-        let bare = match mem::replace(self, GenericZipWriter::Closed) {
-            GenericZipWriter::Storer(w) => w,
+        let bare: &mut W = match self {
+            Storer(w) => &mut unsafe { mem::transmute_copy::<W, W>(w) },
             #[cfg(any(
                 feature = "deflate",
                 feature = "deflate-miniz",
                 feature = "deflate-zlib"
             ))]
-            GenericZipWriter::Deflater(w) => w.finish()?,
+            GenericZipWriter::Deflater(w) => &mut w.finish()?,
             #[cfg(feature = "bzip2")]
-            GenericZipWriter::Bzip2(w) => w.finish()?,
+            GenericZipWriter::Bzip2(w) => &mut w.finish()?,
             #[cfg(feature = "zstd")]
-            GenericZipWriter::Zstd(w) => w.finish()?,
-            GenericZipWriter::Closed => {
+            GenericZipWriter::Zstd(w) => &mut w.finish()?,
+            Closed => {
                 return Err(io::Error::new(
                     io::ErrorKind::BrokenPipe,
                     "ZipWriter was already closed",
@@ -981,81 +991,87 @@ impl<W: Write + Seek> GenericZipWriter<W> {
             }
         };
 
-        *self = {
-            #[allow(deprecated)]
-            match compression {
-                CompressionMethod::Stored => {
-                    if compression_level.is_some() {
-                        return Err(ZipError::UnsupportedArchive(
-                            "Unsupported compression level",
-                        ));
-                    }
-
-                    GenericZipWriter::Storer(bare)
+        #[allow(deprecated)]
+        match compression {
+            CompressionMethod::Stored => {
+                if compression_level.is_some() {
+                    return Err(ZipError::UnsupportedArchive(
+                        "Unsupported compression level",
+                    ));
                 }
-                #[cfg(any(
-                    feature = "deflate",
-                    feature = "deflate-miniz",
-                    feature = "deflate-zlib"
-                ))]
-                CompressionMethod::Deflated => GenericZipWriter::Deflater(DeflateEncoder::new(
-                    bare,
+
+                let _ = mem::replace(self, Storer(unsafe { mem::transmute_copy::<W, W>(bare) }));
+                Ok(())
+            }
+            #[cfg(any(
+            feature = "deflate",
+            feature = "deflate-miniz",
+            feature = "deflate-zlib"
+            ))]
+            CompressionMethod::Deflated => {
+                let _ = mem::replace(self, GenericZipWriter::Deflater(DeflateEncoder::new(
+                    unsafe { mem::transmute_copy::<W, W>(bare) },
                     flate2::Compression::new(
                         clamp_opt(
                             compression_level
                                 .unwrap_or(flate2::Compression::default().level() as i32),
                             deflate_compression_level_range(),
                         )
-                        .ok_or(ZipError::UnsupportedArchive(
-                            "Unsupported compression level",
-                        ))? as u32,
+                            .ok_or(ZipError::UnsupportedArchive(
+                                "Unsupported compression level",
+                            ))? as u32,
                     ),
-                )),
-                #[cfg(feature = "bzip2")]
-                CompressionMethod::Bzip2 => GenericZipWriter::Bzip2(BzEncoder::new(
-                    bare,
+                )));
+                Ok(())
+            },
+            #[cfg(feature = "bzip2")]
+            CompressionMethod::Bzip2 => {
+                let _ = mem::replace(self, GenericZipWriter::Bzip2(BzEncoder::new(
+                    unsafe { mem::transmute_copy::<W, W>(bare) },
                     bzip2::Compression::new(
                         clamp_opt(
                             compression_level
                                 .unwrap_or(bzip2::Compression::default().level() as i32),
                             bzip2_compression_level_range(),
                         )
-                        .ok_or(ZipError::UnsupportedArchive(
-                            "Unsupported compression level",
-                        ))? as u32,
+                            .ok_or(ZipError::UnsupportedArchive(
+                                "Unsupported compression level",
+                            ))? as u32,
                     ),
-                )),
-                CompressionMethod::AES => {
-                    return Err(ZipError::UnsupportedArchive(
-                        "AES compression is not supported for writing",
-                    ))
-                }
-                #[cfg(feature = "zstd")]
-                CompressionMethod::Zstd => GenericZipWriter::Zstd(
+                )));
+                Ok(())
+            },
+            CompressionMethod::AES => {
+                return Err(ZipError::UnsupportedArchive(
+                    "AES compression is not supported for writing",
+                ))
+            }
+            #[cfg(feature = "zstd")]
+            CompressionMethod::Zstd => {
+                let _ = mem::replace(self, GenericZipWriter::Zstd(
                     ZstdEncoder::new(
-                        bare,
+                        unsafe { mem::transmute_copy::<W, W>(bare) },
                         clamp_opt(
                             compression_level.unwrap_or(zstd::DEFAULT_COMPRESSION_LEVEL),
                             zstd::compression_level_range(),
                         )
-                        .ok_or(ZipError::UnsupportedArchive(
-                            "Unsupported compression level",
-                        ))?,
+                            .ok_or(ZipError::UnsupportedArchive(
+                                "Unsupported compression level",
+                            ))?,
                     )
-                    .unwrap(),
-                ),
-                CompressionMethod::Unsupported(..) => {
-                    return Err(ZipError::UnsupportedArchive("Unsupported compression"))
-                }
+                        .unwrap(),
+                ));
+                Ok(())
+            },
+            CompressionMethod::Unsupported(..) => {
+                return Err(ZipError::UnsupportedArchive("Unsupported compression"));
             }
-        };
-
-        Ok(())
+        }
     }
 
     fn ref_mut(&mut self) -> Option<&mut dyn Write> {
         match *self {
-            GenericZipWriter::Storer(ref mut w) => Some(w as &mut dyn Write),
+            Storer(ref mut w) => Some(w as &mut dyn Write),
             #[cfg(any(
                 feature = "deflate",
                 feature = "deflate-miniz",
@@ -1066,7 +1082,7 @@ impl<W: Write + Seek> GenericZipWriter<W> {
             GenericZipWriter::Bzip2(ref mut w) => Some(w as &mut dyn Write),
             #[cfg(feature = "zstd")]
             GenericZipWriter::Zstd(ref mut w) => Some(w as &mut dyn Write),
-            GenericZipWriter::Closed => None,
+            Closed => None,
         }
     }
 
@@ -1076,7 +1092,7 @@ impl<W: Write + Seek> GenericZipWriter<W> {
 
     fn get_plain(&mut self) -> &mut W {
         match *self {
-            GenericZipWriter::Storer(ref mut w) => w,
+            Storer(ref mut w) => w,
             _ => panic!("Should have switched to stored beforehand"),
         }
     }