From 54fa948cfd70de274521e420779d69a8d4eb79a3 Mon Sep 17 00:00:00 2001 From: grom Date: Mon, 26 Oct 2026 07:18:31 -0600 Subject: [PATCH] A BLP level whose span runs past the file ends the chain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `level_data` asked `bytes.get(off..off + sz)` and turned a miss into `Error::OutOfBounds`, which fails the whole decode. Two faults in one line: a level table may name a span past the file, and `off + sz` may overflow — the sizes that do this are wrapped values like `0xFFFFD0C4`, which on a 32-bit `usize` wraps the sum back inside the buffer and hands the caller another level's bytes. Now a deeper level that cannot be sliced ends the chain, as a zero size or offset already does; level 0 still errors, because there is no image without it. The sum is `checked_add`. Measured over this install's chain (138,540 BLP headers, read independently of this decoder): 316 copies declare a level whose span runs past the file. 282 of those levels sit past `max(log2 w, log2 h)`, so the decoder never reads them. 34 do not — Troll facial hair and `item\TextureComponents\` armour art at 128x32 with a wrapped level 6 — and every one of those 34 is shadowed by an intact copy in a higher patch, so nothing on this install decodes differently today. What the fix buys is that the layering is no longer load-bearing: the shadowing is a property of which patches this realm happens to ship, and a level that runs past the file now costs the levels below it rather than the image. Ported from benilla-bourbon (tripleslash/benilla-bourbon, "a BLP mip past the file's end ends the chain"), which measured it on patch-A's `Character\Troll\SCALPUPPERHAIR00_01.blp` — level 6 at the file's end over six intact levels. That file is in this chain too, where `Patch-P.mpq` shadows it with an intact 99,476-byte copy. The test is theirs, unchanged. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn --- crates/benilla-blp/src/lib.rs | 39 +++++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/crates/benilla-blp/src/lib.rs b/crates/benilla-blp/src/lib.rs index 5c567df6..9de7a8d6 100644 --- a/crates/benilla-blp/src/lib.rs +++ b/crates/benilla-blp/src/lib.rs @@ -217,16 +217,23 @@ fn parse_header(bytes: &[u8]) -> Result> { impl Header<'_> { /// Level `level`'s stored bytes, or `None` where the chain ends before the formula's count. + /// + /// Deviation: a deeper level whose span runs past the file also ends the chain, where the + /// reference reads past its buffer. patch-A's `Character\Troll\SCALPUPPERHAIR00_01.blp` ships + /// level 6 at the file's end with size `0xFFFFD0C4`, over six intact levels. `off + sz` is + /// also checked for overflow rather than wrapped: that size wraps the sum back into the + /// buffer, which would have handed the caller a slice of the wrong level's bytes. fn level_data<'b>(&self, bytes: &'b [u8], level: usize) -> Result> { let off = self.offsets[level] as usize; let sz = self.sizes[level] as usize; if level > 0 && (sz == 0 || off == 0) { return Ok(None); } - bytes - .get(off..off + sz) - .map(Some) - .ok_or(Error::OutOfBounds { level }) + match off.checked_add(sz).and_then(|end| bytes.get(off..end)) { + Some(data) => Ok(Some(data)), + None if level > 0 => Ok(None), + None => Err(Error::OutOfBounds { level }), + } } /// DXTC level `level` as the reference reads it: `bpp·max(4,w)·max(4,h)/8` bytes from the @@ -484,6 +491,30 @@ mod tests { assert!(matches!(decode(&b), Err(Error::DimensionsTooLarge { .. }))); } + /// patch-A's `Character\Troll\SCALPUPPERHAIR00_01.blp`: a deeper level at the file's end with a + /// wrapped size ends the chain on the intact levels; the same fault on level 0 still errors. + #[test] + fn a_deeper_level_past_the_file_ends_the_chain() { + // Raw1 4x4 without alpha: levels 4x4, 2x2, 1x1, one index byte per texel. + let base = (HEADER_SIZE + PALETTE_SIZE) as u32; + let mut offsets = [0u32; 16]; + let mut sizes = [0u32; 16]; + (offsets[0], sizes[0]) = (base, 16); + (offsets[1], sizes[1]) = (base + 16, 4); + (offsets[2], sizes[2]) = (base + 20, 0xFFFF_D0C4); + let mut b = header(1, 0, 8, 1, 4, 4, offsets, sizes); + b.resize((base + 20) as usize, 0); + + let d = decode(&b).expect("the intact levels decode"); + assert_eq!(d.mips.len(), 2, "the chain ends before the broken level"); + assert_eq!(decode_native(&b).expect("native too").mips.len(), 2); + + sizes[0] = 0xFFFF_D0C4; + let mut b = header(1, 0, 8, 1, 4, 4, offsets, sizes); + b.resize((base + 20) as usize, 0); + assert!(matches!(decode(&b), Err(Error::OutOfBounds { level: 0 }))); + } + #[test] fn truncated_header_errors_cleanly() { let mut b = vec![0u8; 10];