Bug fixes: file can't be ZIP64 if CDR start is after CDR end

This commit is contained in:
Chris Hennick 2023-05-11 09:33:06 -07:00
parent 61502b22a7
commit 8c6816fb33
No known key found for this signature in database
GPG key ID: 25653935CC8B6C74
3 changed files with 66 additions and 6 deletions

View file

@ -351,6 +351,9 @@ impl<R: Read + Seek> ZipArchive<R> {
.ok_or(ZipError::InvalidArchive( .ok_or(ZipError::InvalidArchive(
"Invalid central directory size or offset", "Invalid central directory size or offset",
))?; ))?;
if directory_start > search_upper_bound {
return Err(ZipError::InvalidArchive("Invalid central directory size or offset"));
}
if footer64.disk_number != footer64.disk_with_central_directory { if footer64.disk_number != footer64.disk_with_central_directory {
return unsupported_zip_error("Support for multi-disk files is not implemented"); return unsupported_zip_error("Support for multi-disk files is not implemented");

View file

@ -61,14 +61,14 @@ impl CentralDirectoryEnd {
}) })
} }
pub fn find_and_parse<T: Read + io::Seek>( pub fn find_and_parse<T: Read + Seek>(
reader: &mut T, reader: &mut T,
) -> ZipResult<(CentralDirectoryEnd, u64)> { ) -> ZipResult<(CentralDirectoryEnd, u64)> {
const HEADER_SIZE: u64 = 22; const HEADER_SIZE: u64 = 22;
const BYTES_BETWEEN_MAGIC_AND_COMMENT_SIZE: u64 = HEADER_SIZE - 6; const BYTES_BETWEEN_MAGIC_AND_COMMENT_SIZE: u64 = HEADER_SIZE - 6;
let file_length = reader.seek(io::SeekFrom::End(0))?; let file_length = reader.seek(io::SeekFrom::End(0))?;
let search_upper_bound = file_length.saturating_sub(HEADER_SIZE + ::std::u16::MAX as u64); let search_upper_bound = file_length.saturating_sub(HEADER_SIZE + u16::MAX as u64);
if file_length < HEADER_SIZE { if file_length < HEADER_SIZE {
return Err(ZipError::InvalidArchive("Invalid zip header")); return Err(ZipError::InvalidArchive("Invalid zip header"));
@ -155,14 +155,14 @@ pub struct Zip64CentralDirectoryEnd {
} }
impl Zip64CentralDirectoryEnd { impl Zip64CentralDirectoryEnd {
pub fn find_and_parse<T: Read + io::Seek>( pub fn find_and_parse<T: Read + Seek>(
reader: &mut T, reader: &mut T,
nominal_offset: u64, nominal_offset: u64,
search_upper_bound: u64, search_upper_bound: u64,
) -> ZipResult<(Zip64CentralDirectoryEnd, u64)> { ) -> ZipResult<(Zip64CentralDirectoryEnd, u64)> {
let mut pos = nominal_offset; let mut pos = search_upper_bound;
while pos <= search_upper_bound { while pos >= nominal_offset {
reader.seek(io::SeekFrom::Start(pos))?; reader.seek(io::SeekFrom::Start(pos))?;
if reader.read_u32::<LittleEndian>()? == ZIP64_CENTRAL_DIRECTORY_END_SIGNATURE { if reader.read_u32::<LittleEndian>()? == ZIP64_CENTRAL_DIRECTORY_END_SIGNATURE {
@ -195,7 +195,7 @@ impl Zip64CentralDirectoryEnd {
)); ));
} }
pos += 1; pos -= 1;
} }
Err(ZipError::InvalidArchive( Err(ZipError::InvalidArchive(

View file

@ -1481,6 +1481,7 @@ mod test {
use crate::ZipArchive; use crate::ZipArchive;
use std::io; use std::io;
use std::io::{Read, Write}; use std::io::{Read, Write};
use zstd::zstd_safe::WriteBuf;
#[test] #[test]
fn write_empty_zip() { fn write_empty_zip() {
@ -1732,6 +1733,62 @@ mod test {
.expect_err("Expected duplicate filename not to be allowed"); .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] #[test]
fn path_to_string() { fn path_to_string() {
let mut path = std::path::PathBuf::new(); let mut path = std::path::PathBuf::new();