From dc351196e2a13f1e1463fefc8ae06f08337ca957 Mon Sep 17 00:00:00 2001 From: Chris Hennick Date: Thu, 11 May 2023 18:52:41 -0700 Subject: [PATCH] Bug fix: don't allow writing files with certain ZIP64 magic strings in their names --- src/read.rs | 38 +++++++-- src/spec.rs | 4 +- src/write.rs | 100 ++++++----------------- tests/data/zip64_magic_in_filename_1.zip | Bin 0 -> 81 bytes tests/data/zip64_magic_in_filename_2.zip | Bin 0 -> 81 bytes tests/data/zip64_magic_in_filename_3.zip | Bin 0 -> 81 bytes 6 files changed, 56 insertions(+), 86 deletions(-) create mode 100644 tests/data/zip64_magic_in_filename_1.zip create mode 100644 tests/data/zip64_magic_in_filename_2.zip create mode 100644 tests/data/zip64_magic_in_filename_3.zip diff --git a/src/read.rs b/src/read.rs index a47586ea..e7a1b192 100644 --- a/src/read.rs +++ b/src/read.rs @@ -1123,6 +1123,10 @@ pub fn read_zipfile_from_stream<'a, R: Read>(reader: &'a mut R) -> ZipResult, } } +use crate::result::ZipError::InvalidArchive; use crate::write::GenericZipWriter::{Closed, Storer}; pub use zip_writer::ZipWriter; @@ -300,7 +301,7 @@ impl ZipWriter { ZipArchive::get_directory_counts(&mut readwriter, &footer, cde_start_pos)?; if readwriter.seek(SeekFrom::Start(directory_start)).is_err() { - return Err(ZipError::InvalidArchive( + return Err(InvalidArchive( "Could not seek to start of central directory", )); } @@ -438,6 +439,7 @@ impl ZipWriter { { let header_start = self.inner.get_plain().stream_position()?; let name = name.into(); + Self::validate_name(&name)?; let permissions = options.permissions.unwrap_or(0o100644); let file = ZipFileData { @@ -491,7 +493,7 @@ impl ZipWriter { fn insert_file_data(&mut self, file: ZipFileData) -> ZipResult { let name = &file.file_name; if self.files_by_name.contains_key(name) { - return Err(ZipError::InvalidArchive("Duplicate filename")); + return Err(InvalidArchive("Duplicate filename")); } let name = name.to_owned(); self.files.push(file); @@ -1030,6 +1032,27 @@ impl ZipWriter { self.insert_file_data(dest_data)?; Ok(()) } + fn validate_name(name: &String) -> ZipResult<()> { + for (index, _) in name.match_indices("PK") { + if name.len() >= index + 4 { + let magic_number = name[index..index + 4] + .as_bytes() + .read_u32::()?; + match magic_number { + spec::ZIP64_CENTRAL_DIRECTORY_END_SIGNATURE => { + return Err(InvalidArchive("Filename can't contain ZIP64 end signature")); + } + spec::ZIP64_CENTRAL_DIRECTORY_END_LOCATOR_SIGNATURE => { + return Err(InvalidArchive( + "Filename can't contain ZIP64 end-locator signature", + )); + } + _ => {} + } + } + } + Ok(()) + } } impl Drop for ZipWriter { @@ -1731,79 +1754,6 @@ mod test { .start_file("foo/bar/test", FileOptions::default()) .expect_err("Expected duplicate filename not to be allowed"); } - - #[test] - fn test_filename_looks_like_zip64_locator() { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); - writer - .start_file( - "PK\u{6}\u{7}\0\0\0\u{11}\0\0\0\0\0\0\0\0\0\0\0\0", - FileOptions::default(), - ) - .unwrap(); - let zip = writer.finish().unwrap(); - let _ = ZipArchive::new(zip).unwrap(); - } - - #[test] - fn test_filename_looks_like_zip64_locator_2() { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); - writer - .start_file( - "PK\u{6}\u{6}\0\0\0\0\0\0\0\0\0\0PK\u{6}\u{7}\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0", - FileOptions::default(), - ) - .unwrap(); - let zip = writer.finish().unwrap(); - println!("{:02x?}", zip.get_ref()); - let _ = ZipArchive::new(zip).unwrap(); - } - - #[test] - fn test_filename_looks_like_zip64_locator_2a() { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); - writer - .start_file( - "PK\u{6}\u{6}PK\u{6}\u{7}\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0", - FileOptions::default(), - ) - .unwrap(); - let zip = writer.finish().unwrap(); - println!("{:02x?}", zip.get_ref()); - let _ = ZipArchive::new(zip).unwrap(); - } - - #[test] - fn test_filename_looks_like_zip64_locator_3() { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); - writer - .start_file("\0PK\u{6}\u{6}", FileOptions::default()) - .unwrap(); - writer - .start_file( - "\0\u{4}\0\0PK\u{6}\u{7}\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\u{3}", - FileOptions::default(), - ) - .unwrap(); - let zip = writer.finish().unwrap(); - println!("{:02x?}", zip.get_ref()); - let _ = ZipArchive::new(zip).unwrap(); - } - - #[test] - fn path_to_string() { - let mut path = std::path::PathBuf::new(); - #[cfg(windows)] - path.push(r"C:\"); - #[cfg(unix)] - path.push("/"); - path.push("windows"); - path.push(".."); - path.push("."); - path.push("system32"); - let path_str = super::path_to_string(&path); - assert_eq!(path_str, "windows/system32"); - } } #[cfg(not(feature = "unreserved"))] diff --git a/tests/data/zip64_magic_in_filename_1.zip b/tests/data/zip64_magic_in_filename_1.zip new file mode 100644 index 0000000000000000000000000000000000000000..36e2bd49c894771223b18a3ea5ee3b4018fde0da GIT binary patch literal 81 McmZQzpf2zR005i-OaK4? literal 0 HcmV?d00001 diff --git a/tests/data/zip64_magic_in_filename_2.zip b/tests/data/zip64_magic_in_filename_2.zip new file mode 100644 index 0000000000000000000000000000000000000000..36e2bd49c894771223b18a3ea5ee3b4018fde0da GIT binary patch literal 81 McmZQzpf2zR005i-OaK4? literal 0 HcmV?d00001 diff --git a/tests/data/zip64_magic_in_filename_3.zip b/tests/data/zip64_magic_in_filename_3.zip new file mode 100644 index 0000000000000000000000000000000000000000..36e2bd49c894771223b18a3ea5ee3b4018fde0da GIT binary patch literal 81 McmZQzpf2zR005i-OaK4? literal 0 HcmV?d00001