Validate IWD entry lengths before returning resource bytes

Cargo.lock

@@ -197,6 +197,7 @@ version = "0.1.0"
 dependencies = [
  "asset_transport",
  "serde_json",
+ "zip",
 ]
 
 [[package]]

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

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 {

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"
+    );
+}

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<R: Read>(
+    entry: &mut zip::read::ZipFile<'_, R>,
+    limit: Option<usize>,
+) -> Result<Vec<u8>, 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<PathBuf> {
     let mut dirs = index_cache()
         .read()
@@ -419,8 +456,7 @@ pub fn read_iwd_named(games_root: &Path, want: &str) -> Option<Vec<u8>> {
                     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);
                     }
                 }