diff --git a/src/extra_fields/zipinfo_utf8.rs b/src/extra_fields/zipinfo_utf8.rs index 7119bbfb5..43f422130 100644 --- a/src/extra_fields/zipinfo_utf8.rs +++ b/src/extra_fields/zipinfo_utf8.rs @@ -13,9 +13,9 @@ pub struct UnicodeExtraField { impl UnicodeExtraField { /// Verifies the checksum and returns the content. - pub fn unwrap_valid(self, ascii_field: &[u8]) -> ZipResult> { + pub fn unwrap_valid(self) -> ZipResult> { let mut crc32 = crc32fast::Hasher::new(); - crc32.update(ascii_field); + crc32.update(&*self.content); let actual_crc32 = crc32.finalize(); if self.crc32 != actual_crc32 { return Err(ZipError::InvalidArchive( diff --git a/src/read.rs b/src/read.rs index edb2ea4a6..2e03cbdab 100644 --- a/src/read.rs +++ b/src/read.rs @@ -1295,25 +1295,20 @@ pub(crate) fn parse_single_extra_field( 0x6375 => { // Info-ZIP Unicode Comment Extra Field // APPNOTE 4.6.8 and https://libzip.org/specifications/extrafld.txt - if !file.is_utf8 { - file.file_comment = String::from_utf8( - UnicodeExtraField::try_from_reader(reader, len)? - .unwrap_valid(file.file_comment.as_bytes())? - .into_vec(), - )? - .into(); - } + file.file_comment = String::from_utf8( + UnicodeExtraField::try_from_reader(reader, len)? + .unwrap_valid()? + .into_vec(), + )? + .into(); } 0x7075 => { // Info-ZIP Unicode Path Extra Field // APPNOTE 4.6.9 and https://libzip.org/specifications/extrafld.txt - if !file.is_utf8 { - file.file_name_raw = UnicodeExtraField::try_from_reader(reader, len)? - .unwrap_valid(&file.file_name_raw)?; - file.file_name = - String::from_utf8(file.file_name_raw.clone().into_vec())?.into_boxed_str(); - file.is_utf8 = true; - } + file.file_name_raw = UnicodeExtraField::try_from_reader(reader, len)?.unwrap_valid()?; + file.file_name = + String::from_utf8(file.file_name_raw.clone().into_vec())?.into_boxed_str(); + file.is_utf8 = true; } _ => { // Other fields are ignored diff --git a/src/write.rs b/src/write.rs index fb77822b4..9e1a0f7cb 100644 --- a/src/write.rs +++ b/src/write.rs @@ -474,7 +474,6 @@ impl<'k> FileOptions<'k, ExtendedFileOptions> { data: &[u8], central_only: bool, ) -> ZipResult<()> { - self.validate_extra_data(data)?; let len = data.len() + 4; if self.extended_options.extra_data.len() + self.extended_options.central_extra_data.len() @@ -502,6 +501,8 @@ impl<'k> FileOptions<'k, ExtendedFileOptions> { vec.write_u16_le(header_id)?; vec.write_u16_le(data.len() as u16)?; vec.write_all(data)?; + let vec_copy = vec.to_owned(); + self.validate_extra_data(&vec_copy)?; Ok(()) } } @@ -860,6 +861,9 @@ impl ZipWriter { Arc::get_mut(extra_field.insert(new)).unwrap() } }; + if let Some(central_only) = options.extended_options.central_extra_data() { + extra_data.extend(central_only.iter()); + } if extra_data.len() + aes_dummy_extra_data.len() > u16::MAX as usize { let _ = self.abort_file(); @@ -872,7 +876,6 @@ impl ZipWriter { // file. extra_data.write_all(&aes_dummy_extra_data)?; } - { let header_start = self.inner.get_plain().stream_position()?; @@ -931,7 +934,10 @@ impl ZipWriter { let align = options.alignment as u64; let unaligned_header_bytes = header_end % align; if unaligned_header_bytes != 0 { - let pad_length = (align - unaligned_header_bytes) as usize; + let mut pad_length = (align - unaligned_header_bytes) as usize; + if pad_length < 4 { + pad_length += align as usize; + } let Some(new_extra_field_length) = (pad_length as u16).checked_add(block.extra_field_length) else { @@ -940,23 +946,15 @@ impl ZipWriter { "Extra data field would be larger than allowed after aligning", )); }; - if pad_length >= 4 { - // Add an extra field to the extra_data, per APPNOTE 4.6.11 - let mut pad_body = vec![0; pad_length - 4]; - if pad_body.len() >= 2 { - [pad_body[0], pad_body[1]] = options.alignment.to_le_bytes(); - } - writer.write_u16_le(0xa11e)?; - writer - .write_u16_le(pad_body.len() as u16) - .map_err(ZipError::from)?; - writer.write_all(&pad_body).map_err(ZipError::from)?; - } else { - // extra_data padding is too small for an extra field header, so pad with - // zeroes - let pad = vec![0; pad_length]; - writer.write_all(&pad).map_err(ZipError::from)?; - } + // Add an extra field to the extra_data, per APPNOTE 4.6.11 + let mut pad_body = vec![0; pad_length - 4]; + debug_assert!(pad_body.len() >= 2); + [pad_body[0], pad_body[1]] = options.alignment.to_le_bytes(); + writer.write_u16_le(0xa11e)?; + writer + .write_u16_le(pad_body.len() as u16) + .map_err(ZipError::from)?; + writer.write_all(&pad_body).map_err(ZipError::from)?; header_end = writer.stream_position()?; // Update extra field length in local file header. @@ -2601,4 +2599,50 @@ mod test { assert!(writer.start_file_from_path("", options).is_err()); Ok(()) } + + #[test] + fn test_fuzz_crash_2024_06_13() -> ZipResult<()> { + use crate::write::ExtendedFileOptions; + let mut writer = ZipWriter::new(Cursor::new(Vec::new())); + writer.set_flush_on_finish_file(false); + let options = FileOptions { + compression_method: Stored, + compression_level: None, + last_modified_time: DateTime::from_date_and_time(2021, 8, 8, 1, 0, 29)?, + permissions: None, + large_file: true, + encrypt_with: None, + extended_options: ExtendedFileOptions { + extra_data: vec![].into(), + central_extra_data: vec![ + 1, 41, 4, 0, 1, 255, 245, 117, 117, 112, 5, 0, 80, 255, 149, 255, 247, + ] + .into(), + }, + alignment: 4103, + zopfli_buffer_size: None, + }; + writer.start_file_from_path("", options)?; + let finished = writer.finish_into_readable()?; + let inner = finished.into_inner(); + writer = ZipWriter::new_append(inner)?; + let options = FileOptions { + compression_method: Stored, + compression_level: None, + last_modified_time: DateTime::default(), + permissions: None, + large_file: false, + encrypt_with: None, + extended_options: ExtendedFileOptions { + extra_data: vec![].into(), + central_extra_data: vec![].into(), + }, + alignment: 0, + zopfli_buffer_size: None, + }; + writer.start_file_from_path("", options)?; + writer.shallow_copy_file_from_path("", "")?; + let _ = writer.finish_into_readable()?; + Ok(()) + } }