A BLP level whose span runs past the file ends the chain
`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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
This commit is contained in:
@@ -217,16 +217,23 @@ fn parse_header(bytes: &[u8]) -> Result<Header<'_>> {
|
||||
|
||||
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<Option<&'b [u8]>> {
|
||||
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];
|
||||
|
||||
Reference in New Issue
Block a user