From abe1b4e91c670106a0397b8990eb363c5bd0b8dd Mon Sep 17 00:00:00 2001 From: Bulat Fatykh <68301540+gitoutofthere@users.noreply.github.com> Date: Thu, 1 Oct 2026 17:46:39 +0300 Subject: [PATCH] Validate IWD entry lengths before returning resource bytes --- Cargo.lock | 1 + crates/approved_tests/Cargo.toml | 3 + crates/approved_tests/src/main.rs | 2 + crates/approved_tests/src/resource_reads.rs | 203 ++++++++++++++++++++ crates/asset_transport/src/iwd.rs | 58 ++++-- 5 files changed, 256 insertions(+), 11 deletions(-) create mode 100644 crates/approved_tests/src/resource_reads.rs diff --git a/Cargo.lock b/Cargo.lock index 828bba6..a2da4c1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -197,6 +197,7 @@ version = "0.1.0" dependencies = [ "asset_transport", "serde_json", + "zip", ] [[package]] diff --git a/crates/approved_tests/Cargo.toml b/crates/approved_tests/Cargo.toml index f20d9ff..ed89737 100644 --- a/crates/approved_tests/Cargo.toml +++ b/crates/approved_tests/Cargo.toml @@ -10,5 +10,8 @@ description = "The owner-approved end-to-end scenarios, run against a real iw4l asset_transport.workspace = true serde_json.workspace = true +[dev-dependencies] +zip.workspace = true + [lints] workspace = true diff --git a/crates/approved_tests/src/main.rs b/crates/approved_tests/src/main.rs index b8162d6..569d9dc 100644 --- a/crates/approved_tests/src/main.rs +++ b/crates/approved_tests/src/main.rs @@ -1,6 +1,8 @@ //! The owner-approved end-to-end scenarios; see README.md. mod report; +#[cfg(test)] +mod resource_reads; mod runner; mod scenario; mod scenarios { diff --git a/crates/approved_tests/src/resource_reads.rs b/crates/approved_tests/src/resource_reads.rs new file mode 100644 index 0000000..7941651 --- /dev/null +++ b/crates/approved_tests/src/resource_reads.rs @@ -0,0 +1,203 @@ +//! Resource-read regressions requested alongside the IWD completeness fix. + +use std::io::{Cursor, Write}; +use std::path::{Path, PathBuf}; +use std::sync::atomic::{AtomicUsize, Ordering}; + +use asset_transport::{IwdIndex, IwdSoundIndex, read_iwd_named}; +use zip::CompressionMethod; +use zip::write::SimpleFileOptions; + +struct FixtureDir(PathBuf); + +impl FixtureDir { + fn new() -> Self { + static NEXT: AtomicUsize = AtomicUsize::new(0); + let nonce = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos(); + let path = std::env::temp_dir().join(format!( + "iw4l-resource-reads-{}-{nonce}-{}", + std::process::id(), + NEXT.fetch_add(1, Ordering::Relaxed) + )); + std::fs::create_dir(&path).unwrap(); + Self(path) + } +} + +impl Drop for FixtureDir { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } +} + +// All payloads are synthetic. Keep the compressed data and its CRC valid, +// changing only the advertised uncompressed size (or explicitly the CRC). +fn write_entry( + path: &Path, + name: &str, + payload: &[u8], + method: CompressionMethod, + declared: u64, + bad_crc: bool, +) { + let mut writer = zip::ZipWriter::new(Cursor::new(Vec::new())); + writer + .start_file(name, SimpleFileOptions::default().compression_method(method)) + .unwrap(); + writer.write_all(payload).unwrap(); + let mut bytes = writer.finish().unwrap().into_inner(); + let central = bytes.windows(4).position(|b| b == b"PK\x01\x02").unwrap(); + if let Ok(size) = u32::try_from(declared) { + bytes[22..26].copy_from_slice(&size.to_le_bytes()); + bytes[central + 24..central + 28].copy_from_slice(&size.to_le_bytes()); + } else { + // A ZIP64 uncompressed-size field, without allocating a large payload. + bytes[central + 24..central + 28].copy_from_slice(&u32::MAX.to_le_bytes()); + bytes[central + 30..central + 32].copy_from_slice(&12u16.to_le_bytes()); + let extra_at = central + 46 + name.len(); + let mut extra = vec![1, 0, 8, 0]; + extra.extend_from_slice(&declared.to_le_bytes()); + drop(bytes.splice(extra_at..extra_at, extra)); + let end = bytes.windows(4).position(|b| b == b"PK\x05\x06").unwrap(); + let central_size = u32::from_le_bytes(bytes[end + 12..end + 16].try_into().unwrap()); + bytes[end + 12..end + 16].copy_from_slice(&(central_size + 12).to_le_bytes()); + } + if bad_crc { + bytes[14] ^= 1; + bytes[central + 16] ^= 1; + } + std::fs::write(path, bytes).unwrap(); +} + +#[test] +fn iwd_payload_length_is_checked_for_images_sounds_and_named_reads() { + for method in [CompressionMethod::Stored, CompressionMethod::Deflated] { + for declared in [2, 4, 8] { + let dir = FixtureDir::new(); + write_entry( + &dir.0.join("image.iwd"), + "images/probe.iwi", + b"1234", + method, + declared, + false, + ); + write_entry( + &dir.0.join("sound.iwd"), + "sound/probe.wav", + b"1234", + method, + declared, + false, + ); + let index = IwdIndex::open(&dir.0).unwrap(); + let image = &index.image_candidates("PROBE").unwrap()[0]; + let sound = IwdSoundIndex::open(&dir.0).unwrap(); + if declared == 4 { + assert_eq!(image.read().unwrap(), b"1234"); + assert_eq!(sound.read_sound("PROBE.WAV").unwrap().unwrap(), b"1234"); + assert_eq!(read_iwd_named(&dir.0, "IMAGES\\probe.iwi").unwrap(), b"1234"); + } else { + assert!(image.read().unwrap_err().contains("length mismatch")); + assert!( + sound + .read_sound("probe.wav") + .unwrap() + .unwrap_err() + .contains("length mismatch") + ); + assert!(read_iwd_named(&dir.0, "images/probe.iwi").is_none()); + } + } + } +} + +#[test] +fn iwd_header_prefix_requires_its_bytes_and_checks_crc_at_entry_end() { + let dir = FixtureDir::new(); + write_entry( + &dir.0.join("valid.iwd"), + "images/valid.iwi", + b"1234", + CompressionMethod::Stored, + 4, + false, + ); + write_entry( + &dir.0.join("short.iwd"), + "images/short.iwi", + b"1234", + CompressionMethod::Stored, + 8, + false, + ); + write_entry( + &dir.0.join("crc.iwd"), + "images/crc.iwi", + b"1234", + CompressionMethod::Stored, + 4, + true, + ); + let index = IwdIndex::open(&dir.0).unwrap(); + let valid = &index.image_candidates("valid").unwrap()[0]; + for limit in [0, 2, 4, 5] { + assert_eq!(valid.read_header(limit).unwrap(), &b"1234"[..limit.min(4)]); + } + let short = &index.image_candidates("short").unwrap()[0]; + assert_eq!(short.read_header(2).unwrap(), b"12"); + for limit in [6, 8, 32] { + assert!(short.read_header(limit).unwrap_err().contains("length mismatch")); + } + let crc = &index.image_candidates("crc").unwrap()[0]; + assert_eq!(crc.read_header(2).unwrap(), b"12"); + assert!(crc.read_header(4).is_err()); + assert!(crc.read_header(5).is_err()); + assert!(crc.read().is_err()); +} + +#[test] +fn iwd_zip64_size_is_rejected_for_payload_but_does_not_truncate_prefix() { + let dir = FixtureDir::new(); + write_entry( + &dir.0.join("huge.iwd"), + "images/huge.iwi", + b"1234", + CompressionMethod::Stored, + u64::MAX, + false, + ); + let index = IwdIndex::open(&dir.0).unwrap(); + let image = &index.image_candidates("huge").unwrap()[0]; + assert!(image.read().is_err()); + assert_eq!(image.read_header(2).unwrap(), b"12"); + assert!(read_iwd_named(&dir.0, "images/huge.iwi").is_none()); +} + +#[test] +fn iwd_named_read_skips_incomplete_candidate_in_existing_archive_order() { + let dir = FixtureDir::new(); + write_entry( + &dir.0.join("a.iwd"), + "default_xboxlive.cfg", + b"set x 1\n", + CompressionMethod::Stored, + 12, + false, + ); + write_entry( + &dir.0.join("b.iwd"), + "default_xboxlive.cfg", + b"set x 2\n", + CompressionMethod::Stored, + 8, + false, + ); + assert_eq!( + read_iwd_named(&dir.0, "default_xboxlive.cfg").unwrap(), + b"set x 2\n" + ); +} diff --git a/crates/asset_transport/src/iwd.rs b/crates/asset_transport/src/iwd.rs index d92c757..4c9dd1e 100644 --- a/crates/asset_transport/src/iwd.rs +++ b/crates/asset_transport/src/iwd.rs @@ -313,19 +313,56 @@ fn read_pooled_entry( let mut entry = archive .by_name(entry_name) .map_err(|error| format!("cannot read {entry_name}: {error}"))?; - let want = limit.map_or(entry.size() as usize, |limit| { - limit.min(entry.size() as usize) - }); - let mut bytes = Vec::with_capacity(want); - let read = match limit { - Some(limit) => entry.take(limit as u64).read_to_end(&mut bytes), - None => entry.read_to_end(&mut bytes), - }; - read.map_err(|error| format!("cannot read {entry_name}: {error}"))?; + let bytes = read_entry_bytes(&mut entry, limit) + .map_err(|error| format!("cannot read {entry_name}: {error}"))?; IWD_INFLATE_NS.fetch_add(inflate_at.elapsed().as_nanos() as u64, Ordering::Relaxed); Ok(bytes) } +fn read_entry_bytes( + entry: &mut zip::read::ZipFile<'_, R>, + limit: Option, +) -> Result, String> { + let size = entry.size(); + // Bound a prefix before converting the archive's u64 size to usize. + let want = limit.map_or(size, |limit| size.min(limit as u64)); + let capacity = usize::try_from(want) + .map_err(|_| format!("IWD entry size {want} does not fit in memory"))?; + let mut bytes = Vec::new(); + bytes + .try_reserve_exact(capacity) + .map_err(|error| format!("cannot allocate {want} bytes for IWD entry: {error}"))?; + entry + .by_ref() + .take(want) + .read_to_end(&mut bytes) + .map_err(|error| error.to_string())?; + if bytes.len() != capacity { + return Err(format!( + "IWD entry length mismatch: expected {want} bytes, read {}", + bytes.len() + )); + } + if want == size { + // Take stops at its limit without asking zip for EOF. Probe once to + // validate the CRC and reject extra data without growing the buffer. + let mut extra = [0u8; 1]; + loop { + match entry.read(&mut extra) { + Ok(0) => break, + Ok(_) => { + return Err(format!( + "IWD entry length mismatch: expected {want} bytes, entry has additional data" + )); + } + Err(error) if error.kind() == std::io::ErrorKind::Interrupted => continue, + Err(error) => return Err(error.to_string()), + } + } + } + Ok(bytes) +} + pub fn cached_iwd_dirs() -> Vec { let mut dirs = index_cache() .read() @@ -419,8 +456,7 @@ pub fn read_iwd_named(games_root: &Path, want: &str) -> Option> { continue; }; if entry.name().replace('\\', "/").eq_ignore_ascii_case(&want) { - let mut bytes = Vec::new(); - if entry.read_to_end(&mut bytes).is_ok() { + if let Ok(bytes) = read_entry_bytes(&mut entry, None) { return Some(bytes); } }