Skip to content

Commit

Permalink
fix: ZIP64 header was being written to central header twice
Browse files Browse the repository at this point in the history
  • Loading branch information
Pr0methean committed Jun 14, 2024
1 parent 40093e9 commit a770913
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 10 deletions.
8 changes: 7 additions & 1 deletion src/read.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1245,7 +1245,7 @@ pub(crate) fn parse_extra_field(file: &mut ZipFileData) -> ZipResult<()> {
/* 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)?;
parse_single_extra_field(file, &mut reader, position, false)?;
position = reader.position();
}
Ok(())
Expand All @@ -1255,12 +1255,18 @@ pub(crate) fn parse_single_extra_field<R: Read>(
file: &mut ZipFileData,
reader: &mut R,
bytes_already_read: u64,
disallow_zip64: bool,
) -> ZipResult<()> {
let kind = reader.read_u16_le()?;
let len = reader.read_u16_le()?;
match kind {
// Zip64 extended information extra field
0x0001 => {
if disallow_zip64 {
return Err(InvalidArchive(
"Can't write a custom field using the ZIP64 ID",
));
}
let mut consumed_len = 0;
if len >= 24 || file.uncompressed_size == spec::ZIP64_BYTES_THR {
file.large_file = true;
Expand Down
58 changes: 49 additions & 9 deletions src/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ impl ExtendedFileOptions {
}
};
Self::add_extra_data_unchecked(vec, header_id, data)?;
Self::validate_extra_data(vec)?;
Self::validate_extra_data(vec, 0)?;
Ok(())
}
}
Expand All @@ -323,12 +323,12 @@ impl ExtendedFileOptions {
Ok(())
}

fn validate_extra_data(data: &[u8]) -> ZipResult<()> {
fn validate_extra_data(data: &[u8], reserved: u64) -> ZipResult<()> {
let len = data.len() as u64;
if len == 0 {
return Ok(());
}
if len > u16::MAX as u64 {
if len + reserved > u16::MAX as u64 {
return Err(ZipError::Io(io::Error::new(
io::ErrorKind::Other,
"Extra-data field can't exceed u16::MAX bytes",
Expand Down Expand Up @@ -360,7 +360,7 @@ impl ExtendedFileOptions {
}
data.seek(SeekFrom::Current(-2))?;
}
parse_single_extra_field(&mut ZipFileData::default(), &mut data, pos)?;
parse_single_extra_field(&mut ZipFileData::default(), &mut data, pos, true)?;
pos = data.position();
}
Ok(())
Expand Down Expand Up @@ -927,13 +927,12 @@ impl<W: Write + Seek> ZipWriter<W> {
}
// file name
writer.write_all(&file.file_name_raw)?;
// zip64 extra field
let zip64_start = writer.stream_position()?;
if file.large_file {
write_local_zip64_extra_field(writer, file)?;
}
let header_end = writer.stream_position()?;

file.extra_data_start = Some(writer.stream_position()?);
file.extra_data_start = Some(header_end);
let mut extra_data_end = header_end + extra_data.len() as u64;
if options.alignment > 1 {
let align = options.alignment as u64;
Expand All @@ -960,13 +959,13 @@ impl<W: Write + Seek> ZipWriter<W> {
extra_data_end = writer.stream_position()?;
debug_assert_eq!(extra_data_end % (options.alignment.max(1) as u64), 0);
self.stats.start = extra_data_end;
ExtendedFileOptions::validate_extra_data(&extra_data)?;
ExtendedFileOptions::validate_extra_data(&extra_data, header_end - zip64_start)?;
file.extra_field = Some(extra_data.into());
} else {
self.stats.start = extra_data_end;
}
if let Some(data) = central_extra_data {
ExtendedFileOptions::validate_extra_data(&data)?;
ExtendedFileOptions::validate_extra_data(&data, extra_data_len as u64)?;

Check failure on line 968 in src/write.rs

View workflow job for this annotation

GitHub Actions / style_and_docs (--all-features)

this expression creates a reference which is immediately dereferenced by the compiler

Check failure on line 968 in src/write.rs

View workflow job for this annotation

GitHub Actions / style_and_docs (--no-default-features)

this expression creates a reference which is immediately dereferenced by the compiler

Check failure on line 968 in src/write.rs

View workflow job for this annotation

GitHub Actions / style_and_docs (--all-features)

this expression creates a reference which is immediately dereferenced by the compiler
let central_extra_data_len = extra_data_len + data.len();
if central_extra_data_len > u16::MAX as usize {
self.abort_file()?;
Expand Down Expand Up @@ -2766,4 +2765,45 @@ mod test {
let _ = writer.finish_into_readable()?;
Ok(())
}

#[allow(deprecated)]
#[test]
fn test_fuzz_crash_2024_06_14b() -> ZipResult<()> {
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(2078, 3, 6, 12, 48, 58)?,
permissions: None,
large_file: true,
encrypt_with: None,
extended_options: ExtendedFileOptions {
extra_data: vec![].into(),
central_extra_data: vec![].into(),
},
alignment: 65521,
..Default::default()
};
writer.start_file_from_path("\u{4}\0@\n//\u{c}", options)?;
writer = ZipWriter::new_append(writer.finish()?)?;
writer.abort_file()?;
let options = FileOptions {
compression_method: CompressionMethod::Unsupported(65535),
compression_level: None,
last_modified_time: DateTime::from_date_and_time(2055, 10, 2, 11, 48, 49)?,
permissions: None,
large_file: true,
encrypt_with: None,
extended_options: ExtendedFileOptions {
extra_data: vec![255, 255, 1, 0, 255, 0, 0, 0, 0].into(),
central_extra_data: vec![].into(),
},
alignment: 65535,
..Default::default()
};
writer.add_directory_from_path("", options)?;
let _ = writer.finish_into_readable()?;
Ok(())
}
}

0 comments on commit a770913

Please sign in to comment.