Skip to content

Commit

Permalink
fix: ZIP64 header was being written twice when copying a file
Browse files Browse the repository at this point in the history
  • Loading branch information
Pr0methean committed Jun 15, 2024
1 parent a770913 commit 052f3a1
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 17 deletions.
4 changes: 1 addition & 3 deletions fuzz/fuzz_targets/fuzz_write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,10 @@ impl <'k> Debug for FileOperation<'k> {
for content_slice in contents {
f.write_fmt(format_args!("writer.write_all(&({:?}[..] as [u8]))?;\n", content_slice))?;
}
f.write_str("drop(options);\n")
},
BasicFileOperation::WriteDirectory(options) => {
f.write_fmt(format_args!("let options = {:?};\n\
writer.add_directory_from_path({:?}, options)?;\n\
drop(options);\n",
writer.add_directory_from_path({:?}, options)?;\n",
options, self.path))
},
BasicFileOperation::WriteSymlinkWithTarget {target, options} => {
Expand Down
41 changes: 28 additions & 13 deletions src/read.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1212,9 +1212,11 @@ fn central_header_to_zip_file_inner<R: Read>(
aes_extra_data_start: 0,
extra_fields: Vec::new(),
};

match parse_extra_field(&mut result) {
Ok(..) | Err(ZipError::Io(..)) => {}
Ok(stripped_extra_field) => {
result.extra_field = stripped_extra_field;
}
Err(ZipError::Io(..)) => {}
Err(e) => return Err(e),
}

Expand All @@ -1234,29 +1236,41 @@ fn central_header_to_zip_file_inner<R: Read>(
Ok(result)
}

pub(crate) fn parse_extra_field(file: &mut ZipFileData) -> ZipResult<()> {
pub(crate) fn parse_extra_field(file: &mut ZipFileData) -> ZipResult<Option<Arc<Vec<u8>>>> {
let Some(ref extra_field) = file.extra_field else {
return Ok(());
return Ok(None);
};
let extra_field = extra_field.clone();
let mut processed_extra_field = extra_field.clone();
let len = extra_field.len();
let extra_field = extra_field.to_vec();
let mut reader = io::Cursor::new(extra_field);
let mut reader = io::Cursor::new(&**extra_field);

/* TODO: codify this structure into Zip64ExtraFieldBlock fields! */
let mut position = reader.position();
while (position as usize) < len {
parse_single_extra_field(file, &mut reader, position, false)?;
position = reader.position();
let mut position = reader.position() as usize;
while (position) < len {
let old_position = position;
let remove = parse_single_extra_field(file, &mut reader, position as u64, false)?;
position = reader.position() as usize;
if remove {
let remaining = len - (position - old_position);
if remaining == 0 {
return Ok(None);
}
let mut new_extra_field = Vec::with_capacity(remaining);
new_extra_field.extend_from_slice(&extra_field[0..old_position]);
new_extra_field.extend_from_slice(&extra_field[position..]);
processed_extra_field = Arc::new(new_extra_field);
}
}
Ok(())
Ok(Some(processed_extra_field))
}

pub(crate) fn parse_single_extra_field<R: Read>(
file: &mut ZipFileData,
reader: &mut R,
bytes_already_read: u64,
disallow_zip64: bool,
) -> ZipResult<()> {
) -> ZipResult<bool> {
let kind = reader.read_u16_le()?;
let len = reader.read_u16_le()?;
match kind {
Expand Down Expand Up @@ -1286,6 +1300,7 @@ pub(crate) fn parse_single_extra_field<R: Read>(
return Err(InvalidArchive("ZIP64 extra-data field is the wrong length"));
};
reader.read_exact(&mut vec![0u8; leftover_len])?;
return Ok(true);
}
0x9901 => {
// AES
Expand Down Expand Up @@ -1350,7 +1365,7 @@ pub(crate) fn parse_single_extra_field<R: Read>(
// Other fields are ignored
}
}
Ok(())
Ok(false)
}

/// Methods for retrieving information on zip files
Expand Down
22 changes: 21 additions & 1 deletion src/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ impl<'a> arbitrary::Arbitrary<'a> for FileOptions<'a, ExtendedFileOptions> {
u.arbitrary_loop(Some(0), Some(10), |u| {
options
.add_extra_data(
u16::arbitrary(u)?,
u.int_in_range(2..=u16::MAX)?,
Box::<[u8]>::arbitrary(u)?,
bool::arbitrary(u)?,
)
Expand Down Expand Up @@ -2806,4 +2806,24 @@ mod test {
let _ = writer.finish_into_readable()?;
Ok(())
}

#[test]
fn test_fuzz_crash_2024_06_14c() -> 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 options = FileOptions { compression_method: Stored, compression_level: None, last_modified_time: DateTime::from_date_and_time(2060, 4, 6, 13, 13, 3)?, permissions: None, large_file: true, encrypt_with: None, extended_options: ExtendedFileOptions {extra_data: vec![].into(), central_extra_data: vec![].into()}, alignment: 0, ..Default::default()};
writer.start_file_from_path("\0", options)?;
writer.write_all(&([]))?;
writer = ZipWriter::new_append(writer.finish()?)?;
writer.deep_copy_file_from_path("\0", "")?;
writer
};
writer.merge_archive(sub_writer.finish_into_readable()?)?;
writer.deep_copy_file_from_path("", "_copy")?;
let _ = writer.finish_into_readable()?;
Ok(())
}
}

0 comments on commit 052f3a1

Please sign in to comment.