fix: Always search for data start when opening an archive for append, and reject the header if data appears to start after central directory
This commit is contained in:
Chris Hennick
parent
6f120223b2
commit
4065f0501f
2 changed files with 88 additions and 21 deletions
35
src/read.rs
35
src/read.rs
|
|
@ -284,7 +284,7 @@ fn find_data_start(
|
|||
// If the value was already set in the meantime, ensure it matches (this is probably
|
||||
// unnecessary).
|
||||
Err(_) => {
|
||||
assert_eq!(*data.data_start.get().unwrap(), data_start);
|
||||
debug_assert_eq!(*data.data_start.get().unwrap(), data_start);
|
||||
}
|
||||
}
|
||||
Ok(data_start)
|
||||
|
|
@ -730,25 +730,21 @@ impl<R: Read + Seek> ZipArchive<R> {
|
|||
} else {
|
||||
dir_info.number_of_files
|
||||
};
|
||||
if dir_info.disk_number != dir_info.disk_with_central_directory {
|
||||
return unsupported_zip_error("Support for multi-disk files is not implemented");
|
||||
}
|
||||
let mut files = Vec::with_capacity(file_capacity);
|
||||
reader.seek(io::SeekFrom::Start(dir_info.directory_start))?;
|
||||
for _ in 0..dir_info.number_of_files {
|
||||
let file = central_header_to_zip_file(reader, dir_info.archive_offset)?;
|
||||
let central_end = reader.stream_position()?;
|
||||
find_data_start(&file, reader)?;
|
||||
reader.seek(SeekFrom::Start(central_end))?;
|
||||
files.push(file);
|
||||
}
|
||||
if dir_info.disk_number != dir_info.disk_with_central_directory {
|
||||
unsupported_zip_error("Support for multi-disk files is not implemented")
|
||||
} else {
|
||||
Ok(SharedBuilder {
|
||||
files,
|
||||
offset: dir_info.archive_offset,
|
||||
dir_start: dir_info.directory_start,
|
||||
config,
|
||||
})
|
||||
}
|
||||
Ok(SharedBuilder {
|
||||
files,
|
||||
offset: dir_info.archive_offset,
|
||||
dir_start: dir_info.directory_start,
|
||||
config,
|
||||
})
|
||||
}
|
||||
|
||||
fn sort_result(
|
||||
|
|
@ -1128,7 +1124,15 @@ pub(crate) fn central_header_to_zip_file<R: Read + Seek>(
|
|||
|
||||
// Parse central header
|
||||
let block = ZipCentralEntryBlock::parse(reader)?;
|
||||
central_header_to_zip_file_inner(reader, archive_offset, central_header_start, block)
|
||||
let file = central_header_to_zip_file_inner(reader, archive_offset, central_header_start, block)?;
|
||||
let central_header_end = reader.stream_position()?;
|
||||
let data_start = find_data_start(&file, reader)?;
|
||||
if data_start > central_header_start {
|
||||
return Err(InvalidArchive("A file can't start after its central-directory header"));
|
||||
}
|
||||
file.data_start.get_or_init(|| data_start);
|
||||
reader.seek(SeekFrom::Start(central_header_end))?;
|
||||
Ok(file)
|
||||
}
|
||||
|
||||
#[inline]
|
||||
|
|
@ -1173,7 +1177,6 @@ fn central_header_to_zip_file_inner<R: Read>(
|
|||
let file_name_raw = read_variable_length_byte_field(reader, file_name_length as usize)?;
|
||||
let extra_field = read_variable_length_byte_field(reader, extra_field_length as usize)?;
|
||||
let file_comment_raw = read_variable_length_byte_field(reader, file_comment_length as usize)?;
|
||||
|
||||
let file_name: Box<str> = match is_utf8 {
|
||||
true => String::from_utf8_lossy(&file_name_raw).into(),
|
||||
false => file_name_raw.clone().from_cp437(),
|
||||
|
|
|
|||
74
src/write.rs
74
src/write.rs
|
|
@ -3105,6 +3105,7 @@ mod test {
|
|||
fn test_fuzz_crash_2024_06_17a() -> ZipResult<()> {
|
||||
let mut writer = ZipWriter::new(Cursor::new(Vec::new()));
|
||||
writer.set_flush_on_finish_file(false);
|
||||
const PATH_1: &'static str = "\0I\01\0P\0\0\u{2}\0\0\u{1a}\u{1a}\u{1a}\u{1a}\u{1b}\u{1a}UT\u{5}\0\0\u{1a}\u{1a}\u{1a}\u{1a}UT\u{5}\0\u{1}\0\u{1a}\u{1a}\u{1a}UT\t\0uc\u{5}\0\0\0\0\u{7f}\u{7f}\u{7f}\u{7f}PK\u{6}";
|
||||
let sub_writer = {
|
||||
let mut writer = ZipWriter::new(Cursor::new(Vec::new()));
|
||||
writer.set_flush_on_finish_file(false);
|
||||
|
|
@ -3120,18 +3121,81 @@ mod test {
|
|||
writer.merge_archive(sub_writer.finish_into_readable()?)?;
|
||||
writer = ZipWriter::new_append(writer.finish_into_readable()?.into_inner())?;
|
||||
let options = FileOptions { compression_method: Stored, compression_level: None, last_modified_time: DateTime::from_date_and_time(1980, 11, 14, 10, 46, 47)?, permissions: None, large_file: false, encrypt_with: None, extended_options: ExtendedFileOptions {extra_data: vec![].into(), central_extra_data: vec![].into()}, alignment: 0, ..Default::default() };
|
||||
writer.start_file_from_path("\0I\01\0P\0\0\u{2}\0\0\u{1a}\u{1a}\u{1a}\u{1a}\u{1b}\u{1a}UT\u{5}\0\0\u{1a}\u{1a}\u{1a}\u{1a}UT\u{5}\0\u{1}\0\u{1a}\u{1a}\u{1a}UT\t\0uc\u{5}\0\0\0\0\u{7f}\u{7f}\u{7f}\u{7f}PK\u{6}", options)?;
|
||||
writer.deep_copy_file_from_path("\0I\01\0P\0\0\u{2}\0\0\u{1a}\u{1a}\u{1a}\u{1a}\u{1b}\u{1a}UT\u{5}\0\0\u{1a}\u{1a}\u{1a}\u{1a}UT\u{5}\0\u{1}\0\u{1a}\u{1a}\u{1a}UT\t\0uc\u{5}\0\0\0\0\u{7f}\u{7f}\u{7f}\u{7f}PK\u{6}", "eee\u{6}\0\0\0\0\0\0\0\0\0\0\0$\0\0\0\0\0\0\u{7f}\u{7f}PK\u{6}\u{6}K\u{6}\u{6}\u{6}\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\u{1}\0\0\0\0\0\0\0\0\u{1}\0\0PK\u{1}\u{1e},\0\0\0\0\0\0\0\0\0\0\0\u{8}\0*\0\0\u{1}PK\u{6}\u{7}PK\u{6}\u{6}\0\0\0\0\0\0\0\0}K\u{2}\u{6}")?;
|
||||
writer.start_file_from_path(PATH_1, options)?;
|
||||
writer.deep_copy_file_from_path(PATH_1, "eee\u{6}\0\0\0\0\0\0\0\0\0\0\0$\0\0\0\0\0\0\u{7f}\u{7f}PK\u{6}\u{6}K\u{6}\u{6}\u{6}\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\u{1}\0\0\0\0\0\0\0\0\u{1}\0\0PK\u{1}\u{1e},\0\0\0\0\0\0\0\0\0\0\0\u{8}\0*\0\0\u{1}PK\u{6}\u{7}PK\u{6}\u{6}\0\0\0\0\0\0\0\0}K\u{2}\u{6}")?;
|
||||
writer
|
||||
};
|
||||
writer.merge_archive(sub_writer.finish_into_readable()?)?;
|
||||
writer = ZipWriter::new_append(writer.finish_into_readable()?.into_inner())?;
|
||||
writer.deep_copy_file_from_path("", "copy")?;
|
||||
writer.deep_copy_file_from_path(PATH_1, "")?;
|
||||
writer = ZipWriter::new_append(writer.finish_into_readable()?.into_inner())?;
|
||||
writer.shallow_copy_file_from_path("", "copy")?;
|
||||
writer = ZipWriter::new_append(writer.finish_into_readable()?.into_inner())?;
|
||||
writer.deep_copy_file_from_path("", "copy")?;
|
||||
writer = ZipWriter::new_append(writer.finish_into_readable()?.into_inner())?;
|
||||
let _ = writer.finish_into_readable()?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[cfg(feature = "bzip2")]
|
||||
fn test_fuzz_crash_2024_06_17b() -> 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 sub_writer = {
|
||||
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 sub_writer = {
|
||||
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 sub_writer = {
|
||||
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 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(1981, 1, 1, 0, 0, 21)?, permissions: Some(16908288), large_file: false, encrypt_with: None, extended_options: ExtendedFileOptions {extra_data: vec![].into(), central_extra_data: vec![].into()}, alignment: 20555, ..Default::default() };
|
||||
writer.start_file_from_path("\0\u{7}\u{1}\0\0\0\0\0\0\0\0\u{1}\0\0PK\u{1}\u{2};\u{1a}\u{18}\u{1a}UT\t.........................\0u", options)?;
|
||||
writer
|
||||
};
|
||||
writer.merge_archive(sub_writer.finish_into_readable()?)?;
|
||||
let options = FileOptions { compression_method: CompressionMethod::Bzip2, compression_level: Some(5), last_modified_time: DateTime::from_date_and_time(2055, 7, 7, 3, 6, 6)?, permissions: None, large_file: false, 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\0\0\0..\0\0\0\0\0\u{7f}\u{7f}PK\u{6}\u{6}K\u{6}\u{6}\u{6}\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\u{1}\0\0\0\0\0\0\0\0\u{1}\0\0PK\u{1}\u{1e},\0\0\0\0\0\0\0\0\0\0\0\u{8}\0*\0\0\u{1}PK\u{6}\u{7}PK\u{6}\u{6}\0\0\0\0\0\0\0\0}K\u{2}\u{6}", options)?;
|
||||
writer = ZipWriter::new_append(writer.finish_into_readable()?.into_inner())?;
|
||||
writer
|
||||
};
|
||||
writer.merge_archive(sub_writer.finish_into_readable()?)?;
|
||||
writer = ZipWriter::new_append(writer.finish_into_readable()?.into_inner())?;
|
||||
writer
|
||||
};
|
||||
writer.merge_archive(sub_writer.finish_into_readable()?)?;
|
||||
writer = ZipWriter::new_append(writer.finish_into_readable()?.into_inner())?;
|
||||
writer
|
||||
};
|
||||
writer.merge_archive(sub_writer.finish_into_readable()?)?;
|
||||
writer = ZipWriter::new_append(writer.finish_into_readable()?.into_inner())?;
|
||||
writer
|
||||
};
|
||||
writer.merge_archive(sub_writer.finish_into_readable()?)?;
|
||||
writer
|
||||
};
|
||||
writer.merge_archive(sub_writer.finish_into_readable()?)?;
|
||||
writer
|
||||
};
|
||||
writer.merge_archive(sub_writer.finish_into_readable()?)?;
|
||||
writer
|
||||
};
|
||||
writer.merge_archive(sub_writer.finish_into_readable()?)?;
|
||||
let _ = writer.finish_into_readable()?;
|
||||
Ok(())
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue