From 0bd2fe11d7037d4324679943d09c04cba50de7fe Mon Sep 17 00:00:00 2001 From: Chris Hennick <4961925+Pr0methean@users.noreply.github.com> Date: Thu, 13 Jun 2024 17:48:54 -0700 Subject: [PATCH] fix: Parse the extra field and reject it if invalid --- src/read.rs | 202 ++++++++++++++++++++++++++------------------------- src/types.rs | 5 +- src/write.rs | 111 +++++++++++++++++++--------- 3 files changed, 182 insertions(+), 136 deletions(-) diff --git a/src/read.rs b/src/read.rs index 3e6723c9..edb2ea4a 100644 --- a/src/read.rs +++ b/src/read.rs @@ -1206,109 +1206,17 @@ fn central_header_to_zip_file_inner( Ok(result) } -fn parse_extra_field(file: &mut ZipFileData) -> ZipResult<()> { - let Some(extra_field) = &file.extra_field else { +pub(crate) fn parse_extra_field(file: &mut ZipFileData) -> ZipResult<()> { + let Some(ref extra_field) = file.extra_field else { return Ok(()); }; - let mut reader = io::Cursor::new(extra_field.as_ref()); + let len = extra_field.len(); + let extra_field = extra_field.to_vec(); + let mut reader = io::Cursor::new(extra_field); /* TODO: codify this structure into Zip64ExtraFieldBlock fields! */ - while (reader.position() as usize) < extra_field.len() { - let kind = reader.read_u16_le()?; - let len = reader.read_u16_le()?; - let mut len_left = len as i64; - match kind { - // Zip64 extended information extra field - 0x0001 => { - if file.uncompressed_size == spec::ZIP64_BYTES_THR { - file.large_file = true; - file.uncompressed_size = reader.read_u64_le()?; - len_left -= 8; - } - if file.compressed_size == spec::ZIP64_BYTES_THR { - file.large_file = true; - file.compressed_size = reader.read_u64_le()?; - len_left -= 8; - } - if file.header_start == spec::ZIP64_BYTES_THR { - file.header_start = reader.read_u64_le()?; - len_left -= 8; - } - } - 0x9901 => { - // AES - if len != 7 { - return Err(ZipError::UnsupportedArchive( - "AES extra data field has an unsupported length", - )); - } - let vendor_version = reader.read_u16_le()?; - let vendor_id = reader.read_u16_le()?; - let mut out = [0u8]; - reader.read_exact(&mut out)?; - let aes_mode = out[0]; - let compression_method = CompressionMethod::parse_from_u16(reader.read_u16_le()?); - - if vendor_id != 0x4541 { - return Err(ZipError::InvalidArchive("Invalid AES vendor")); - } - let vendor_version = match vendor_version { - 0x0001 => AesVendorVersion::Ae1, - 0x0002 => AesVendorVersion::Ae2, - _ => return Err(ZipError::InvalidArchive("Invalid AES vendor version")), - }; - match aes_mode { - 0x01 => { - file.aes_mode = Some((AesMode::Aes128, vendor_version, compression_method)) - } - 0x02 => { - file.aes_mode = Some((AesMode::Aes192, vendor_version, compression_method)) - } - 0x03 => { - file.aes_mode = Some((AesMode::Aes256, vendor_version, compression_method)) - } - _ => return Err(ZipError::InvalidArchive("Invalid AES encryption strength")), - }; - file.compression_method = compression_method; - } - 0x5455 => { - // extended timestamp - // https://libzip.org/specifications/extrafld.txt - - file.extra_fields.push(ExtraField::ExtendedTimestamp( - ExtendedTimestamp::try_from_reader(&mut reader, len)?, - )); - - // the reader for ExtendedTimestamp consumes `len` bytes - len_left = 0; - } - 0x6375 => { - // Info-ZIP Unicode Comment Extra Field - // APPNOTE 4.6.8 and https://libzip.org/specifications/extrafld.txt - if !file.is_utf8 { - file.file_comment = String::from_utf8( - UnicodeExtraField::try_from_reader(&mut reader, len)? - .unwrap_valid(file.file_comment.as_bytes())? - .into_vec(), - )? - .into(); - } - } - 0x7075 => { - // Info-ZIP Unicode Path Extra Field - // APPNOTE 4.6.9 and https://libzip.org/specifications/extrafld.txt - if !file.is_utf8 { - file.file_name_raw = UnicodeExtraField::try_from_reader(&mut reader, len)? - .unwrap_valid(&file.file_name_raw)?; - file.file_name = - String::from_utf8(file.file_name_raw.clone().into_vec())?.into_boxed_str(); - file.is_utf8 = true; - } - } - _ => { - // Other fields are ignored - } - } + while (reader.position() as usize) < len { + let len_left = parse_single_extra_field(file, &mut reader)?; // We could also check for < 0 to check for errors if len_left > 0 { @@ -1318,6 +1226,102 @@ fn parse_extra_field(file: &mut ZipFileData) -> ZipResult<()> { Ok(()) } +pub(crate) fn parse_single_extra_field( + file: &mut ZipFileData, + reader: &mut R, +) -> ZipResult { + let kind = reader.read_u16_le()?; + let len = reader.read_u16_le()?; + let mut len_left = len as i64; + match kind { + // Zip64 extended information extra field + 0x0001 => { + if file.uncompressed_size == spec::ZIP64_BYTES_THR { + file.large_file = true; + file.uncompressed_size = reader.read_u64_le()?; + len_left -= 8; + } + if file.compressed_size == spec::ZIP64_BYTES_THR { + file.large_file = true; + file.compressed_size = reader.read_u64_le()?; + len_left -= 8; + } + if file.header_start == spec::ZIP64_BYTES_THR { + file.header_start = reader.read_u64_le()?; + len_left -= 8; + } + } + 0x9901 => { + // AES + if len != 7 { + return Err(ZipError::UnsupportedArchive( + "AES extra data field has an unsupported length", + )); + } + let vendor_version = reader.read_u16_le()?; + let vendor_id = reader.read_u16_le()?; + let mut out = [0u8]; + reader.read_exact(&mut out)?; + let aes_mode = out[0]; + let compression_method = CompressionMethod::parse_from_u16(reader.read_u16_le()?); + + if vendor_id != 0x4541 { + return Err(ZipError::InvalidArchive("Invalid AES vendor")); + } + let vendor_version = match vendor_version { + 0x0001 => AesVendorVersion::Ae1, + 0x0002 => AesVendorVersion::Ae2, + _ => return Err(ZipError::InvalidArchive("Invalid AES vendor version")), + }; + match aes_mode { + 0x01 => file.aes_mode = Some((AesMode::Aes128, vendor_version, compression_method)), + 0x02 => file.aes_mode = Some((AesMode::Aes192, vendor_version, compression_method)), + 0x03 => file.aes_mode = Some((AesMode::Aes256, vendor_version, compression_method)), + _ => return Err(ZipError::InvalidArchive("Invalid AES encryption strength")), + }; + file.compression_method = compression_method; + } + 0x5455 => { + // extended timestamp + // https://libzip.org/specifications/extrafld.txt + + file.extra_fields.push(ExtraField::ExtendedTimestamp( + ExtendedTimestamp::try_from_reader(reader, len)?, + )); + + // the reader for ExtendedTimestamp consumes `len` bytes + len_left = 0; + } + 0x6375 => { + // Info-ZIP Unicode Comment Extra Field + // APPNOTE 4.6.8 and https://libzip.org/specifications/extrafld.txt + if !file.is_utf8 { + file.file_comment = String::from_utf8( + UnicodeExtraField::try_from_reader(reader, len)? + .unwrap_valid(file.file_comment.as_bytes())? + .into_vec(), + )? + .into(); + } + } + 0x7075 => { + // Info-ZIP Unicode Path Extra Field + // APPNOTE 4.6.9 and https://libzip.org/specifications/extrafld.txt + if !file.is_utf8 { + file.file_name_raw = UnicodeExtraField::try_from_reader(reader, len)? + .unwrap_valid(&file.file_name_raw)?; + file.file_name = + String::from_utf8(file.file_name_raw.clone().into_vec())?.into_boxed_str(); + file.is_utf8 = true; + } + } + _ => { + // Other fields are ignored + } + } + Ok(len_left) +} + /// Methods for retrieving information on zip files impl<'a> ZipFile<'a> { fn get_reader(&mut self) -> ZipResult<&mut ZipFileReader<'a>> { diff --git a/src/types.rs b/src/types.rs index d81b4249..d101c75d 100644 --- a/src/types.rs +++ b/src/types.rs @@ -34,11 +34,12 @@ pub(crate) struct ZipRawValues { pub(crate) uncompressed_size: u64, } -#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[derive(Clone, Copy, Debug, PartialEq, Eq, Default)] #[repr(u8)] pub enum System { Dos = 0, Unix = 3, + #[default] Unknown, } @@ -420,7 +421,7 @@ pub const MIN_VERSION: u8 = 10; pub const DEFAULT_VERSION: u8 = 45; /// Structure representing a ZIP file. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Default)] pub struct ZipFileData { /// Compatibility of the file attribute information pub system: System, diff --git a/src/write.rs b/src/write.rs index cc82dcc6..4f75bcd0 100644 --- a/src/write.rs +++ b/src/write.rs @@ -3,7 +3,10 @@ #[cfg(feature = "aes-crypto")] use crate::aes::AesWriter; use crate::compression::CompressionMethod; -use crate::read::{find_content, Config, ZipArchive, ZipFile, ZipFileReader}; +use crate::read::{ + find_content, parse_extra_field, parse_single_extra_field, Config, ZipArchive, ZipFile, + ZipFileReader, +}; use crate::result::{ZipError, ZipResult}; use crate::spec::{self, FixedSizeBlock, Magic}; #[cfg(feature = "aes-crypto")] @@ -21,6 +24,7 @@ use std::default::Default; use std::fmt::{Debug, Formatter}; use std::io; use std::io::prelude::*; +use std::io::Cursor; use std::io::{BufReader, SeekFrom}; use std::marker::PhantomData; use std::mem; @@ -427,6 +431,42 @@ impl<'k, T: FileOptionExtension> FileOptions<'k, T> { } } impl<'k> FileOptions<'k, ExtendedFileOptions> { + fn validate_extra_data(&self, data: &[u8]) -> ZipResult<()> { + if data.len() > u16::MAX as usize { + return Err(ZipError::Io(io::Error::new( + io::ErrorKind::Other, + "Extra-data field can't exceed u16::MAX bytes", + ))); + } + if data.len() < 2 { + return Err(ZipError::Io(io::Error::new( + io::ErrorKind::Other, + "Extra-data field needs 2 tag bytes", + ))); + } + parse_single_extra_field( + &mut ZipFileData::default(), + &mut Cursor::new(data.to_owned()), + )?; + #[cfg(not(feature = "unreserved"))] + { + let header_id = u16::from_le_bytes([data[0], data[1]]); + if header_id <= 31 + || EXTRA_FIELD_MAPPING + .iter() + .any(|&mapped| mapped == header_id) + { + return Err(ZipError::Io(io::Error::new( + io::ErrorKind::Other, + format!( + "Extra data header ID {header_id:#06} requires crate feature \"unreserved\"", + ), + ))); + } + } + Ok(()) + } + /// Adds an extra data field. pub fn add_extra_data( &mut self, @@ -434,7 +474,7 @@ impl<'k> FileOptions<'k, ExtendedFileOptions> { data: &[u8], central_only: bool, ) -> ZipResult<()> { - validate_extra_data(header_id, data)?; + self.validate_extra_data(data)?; let len = data.len() + 4; if self.extended_options.extra_data.len() + self.extended_options.central_extra_data.len() @@ -856,6 +896,7 @@ impl ZipWriter { aes_mode, extra_field, ); + parse_extra_field(&mut file)?; file.version_made_by = file.version_made_by.max(file.version_needed() as u8); let index = self.insert_file_data(file)?; let file = &mut self.files[index]; @@ -1845,39 +1886,6 @@ fn write_central_directory_header(writer: &mut T, file: &ZipFileData) Ok(()) } -fn validate_extra_data(header_id: u16, data: &[u8]) -> ZipResult<()> { - if data.len() > u16::MAX as usize { - return Err(ZipError::Io(io::Error::new( - io::ErrorKind::Other, - "Extra-data field can't exceed u16::MAX bytes", - ))); - } - if header_id == 0x0001 { - return Err(ZipError::Io(io::Error::new( - io::ErrorKind::Other, - "No custom ZIP64 extra data allowed", - ))); - } - - #[cfg(not(feature = "unreserved"))] - { - if header_id <= 31 - || EXTRA_FIELD_MAPPING - .iter() - .any(|&mapped| mapped == header_id) - { - return Err(ZipError::Io(io::Error::new( - io::ErrorKind::Other, - format!( - "Extra data header ID {header_id:#06} requires crate feature \"unreserved\"", - ), - ))); - } - } - - Ok(()) -} - fn write_local_zip64_extra_field(writer: &mut T, file: &ZipFileData) -> ZipResult<()> { // This entry in the Local header MUST include BOTH original // and compressed file size fields. @@ -2560,4 +2568,37 @@ mod test { let _ = writer.finish_into_readable()?; Ok(()) } + + #[test] + #[cfg(feature = "aes-crypto")] + fn test_invalid_extra_data() -> ZipResult<()> { + use crate::write::EncryptWith::Aes; + use crate::write::ExtendedFileOptions; + use crate::AesMode::Aes256; + 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(1980, 1, 4, 6, 54, 0)?, + permissions: None, + large_file: false, + encrypt_with: Some(Aes { + mode: Aes256, + password: "", + }), + extended_options: ExtendedFileOptions { + extra_data: vec![].into(), + central_extra_data: vec![ + 0, 177, 15, 0, 207, 117, 177, 117, 112, 2, 0, 255, 255, 131, 255, 255, 255, 80, + 185, + ] + .into(), + }, + alignment: 32787, + zopfli_buffer_size: None, + }; + assert!(writer.start_file_from_path("", options).is_err()); + Ok(()) + } }