chore: Fix more errors when parsing multiple extra fields

This commit is contained in:
Chris Hennick 2024-06-13 23:11:32 -07:00
parent 290fd97013
commit 6b93d358d5
No known key found for this signature in database
GPG key ID: DA47AABA4961C509
2 changed files with 11 additions and 22 deletions

View file

@ -1216,12 +1216,7 @@ pub(crate) fn parse_extra_field(file: &mut ZipFileData) -> ZipResult<()> {
/* TODO: codify this structure into Zip64ExtraFieldBlock fields! */ /* TODO: codify this structure into Zip64ExtraFieldBlock fields! */
while (reader.position() as usize) < len { while (reader.position() as usize) < len {
let len_left = parse_single_extra_field(file, &mut reader)?; parse_single_extra_field(file, &mut reader)?;
// We could also check for < 0 to check for errors
if len_left > 0 {
reader.seek(io::SeekFrom::Current(len_left))?;
}
} }
Ok(()) Ok(())
} }
@ -1229,26 +1224,22 @@ pub(crate) fn parse_extra_field(file: &mut ZipFileData) -> ZipResult<()> {
pub(crate) fn parse_single_extra_field<R: Read>( pub(crate) fn parse_single_extra_field<R: Read>(
file: &mut ZipFileData, file: &mut ZipFileData,
reader: &mut R, reader: &mut R,
) -> ZipResult<i64> { ) -> ZipResult<()> {
let kind = reader.read_u16_le()?; let kind = reader.read_u16_le()?;
let len = reader.read_u16_le()?; let len = reader.read_u16_le()?;
let mut len_left = len as i64;
match kind { match kind {
// Zip64 extended information extra field // Zip64 extended information extra field
0x0001 => { 0x0001 => {
if file.uncompressed_size == spec::ZIP64_BYTES_THR { if file.uncompressed_size == spec::ZIP64_BYTES_THR {
file.large_file = true; file.large_file = true;
file.uncompressed_size = reader.read_u64_le()?; file.uncompressed_size = reader.read_u64_le()?;
len_left -= 8;
} }
if file.compressed_size == spec::ZIP64_BYTES_THR { if file.compressed_size == spec::ZIP64_BYTES_THR {
file.large_file = true; file.large_file = true;
file.compressed_size = reader.read_u64_le()?; file.compressed_size = reader.read_u64_le()?;
len_left -= 8;
} }
if file.header_start == spec::ZIP64_BYTES_THR { if file.header_start == spec::ZIP64_BYTES_THR {
file.header_start = reader.read_u64_le()?; file.header_start = reader.read_u64_le()?;
len_left -= 8;
} }
} }
0x9901 => { 0x9901 => {
@ -1280,7 +1271,6 @@ pub(crate) fn parse_single_extra_field<R: Read>(
_ => return Err(ZipError::InvalidArchive("Invalid AES encryption strength")), _ => return Err(ZipError::InvalidArchive("Invalid AES encryption strength")),
}; };
file.compression_method = compression_method; file.compression_method = compression_method;
len_left -= 7;
} }
0x5455 => { 0x5455 => {
// extended timestamp // extended timestamp
@ -1289,9 +1279,6 @@ pub(crate) fn parse_single_extra_field<R: Read>(
file.extra_fields.push(ExtraField::ExtendedTimestamp( file.extra_fields.push(ExtraField::ExtendedTimestamp(
ExtendedTimestamp::try_from_reader(reader, len)?, ExtendedTimestamp::try_from_reader(reader, len)?,
)); ));
// the reader for ExtendedTimestamp consumes `len` bytes
len_left = 0;
} }
0x6375 => { 0x6375 => {
// Info-ZIP Unicode Comment Extra Field // Info-ZIP Unicode Comment Extra Field
@ -1315,7 +1302,7 @@ pub(crate) fn parse_single_extra_field<R: Read>(
// Other fields are ignored // Other fields are ignored
} }
} }
Ok(len_left) Ok(())
} }
/// Methods for retrieving information on zip files /// Methods for retrieving information on zip files

View file

@ -318,14 +318,16 @@ impl ExtendedFileOptions {
fn validate_extra_data(&self) -> ZipResult<()> { fn validate_extra_data(&self) -> ZipResult<()> {
let mut data = self.extra_data.to_vec(); let mut data = self.extra_data.to_vec();
data.extend(self.central_extra_data.iter()); data.extend(self.central_extra_data.iter());
if data.len() > u16::MAX as usize { let len = data.len() as u64;
if len > u16::MAX as u64 {
return Err(ZipError::Io(io::Error::new( return Err(ZipError::Io(io::Error::new(
io::ErrorKind::Other, io::ErrorKind::Other,
"Extra-data field can't exceed u16::MAX bytes", "Extra-data field can't exceed u16::MAX bytes",
))); )));
} }
while !data.is_empty() { let mut data = Cursor::new(data);
if data.len() < 2 { while data.position() < len {
if len - data.position() < 2 {
return Err(ZipError::Io(io::Error::new( return Err(ZipError::Io(io::Error::new(
io::ErrorKind::Other, io::ErrorKind::Other,
"Extra-data field needs 2 tag bytes", "Extra-data field needs 2 tag bytes",
@ -333,7 +335,8 @@ impl ExtendedFileOptions {
} }
#[cfg(not(feature = "unreserved"))] #[cfg(not(feature = "unreserved"))]
{ {
let header_id = u16::from_le_bytes([data[0], data[1]]); use crate::unstable::LittleEndianReadExt;
let header_id = data.read_u16_le()?;
if header_id <= 31 if header_id <= 31
|| EXTRA_FIELD_MAPPING || EXTRA_FIELD_MAPPING
.iter() .iter()
@ -347,8 +350,7 @@ impl ExtendedFileOptions {
))); )));
} }
} }
let len_left = parse_single_extra_field(&mut ZipFileData::default(), &mut Cursor::new(data.to_vec()))?; parse_single_extra_field(&mut ZipFileData::default(), &mut data)?;
data = data[(data.len() - len_left as usize)..].to_owned()
} }
Ok(()) Ok(())
} }