From fd8c8c185d38169980c00d517eceac8d65601ed3 Mon Sep 17 00:00:00 2001 From: KenM76 Date: Tue, 27 Oct 2026 05:43:31 -0400 Subject: [PATCH] fix(xfa): repair a BMP offset that points at the header's end with masks or a palette to follow Review follow-up: an offset equal to the end of the info header is only right when no bitfield masks or colour table come after it; otherwise the pixels were read from the masks or the table. It is now repaired like an offset inside the headers. Offsets strictly inside the masks or table are still taken as given (they may come from an overstated biClrUsed). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SqmHYx8FiWM5PcsoiErgp9 --- crates/xfa/src/image.rs | 7 ++++--- crates/xfa/src/tests.rs | 6 ++++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/crates/xfa/src/image.rs b/crates/xfa/src/image.rs index a284b444a8..e41cbd90a9 100644 --- a/crates/xfa/src/image.rs +++ b/crates/xfa/src/image.rs @@ -42,7 +42,7 @@ fn is_bmp(data: &[u8]) -> bool { } /// `data` with a BMP's pixel-data offset (`bfOffBits`) corrected when it points into the file or -/// info header, as some writers leave it at 0: the rows then start after the headers, the colour +/// info header or at its end with masks or a colour table still to come, as some writers leave it at 0: the rows then start after the headers, the colour /// masks and the colour table. The `image` decoder seeks to the declared offset unchecked, so /// such a picture would be read from the wrong place, shifted and wrapped. Anything else, /// including a header too short to measure, is returned as it is. @@ -68,7 +68,8 @@ fn bmp_pixel_offset_repair(data: &[u8]) -> Option { } let (declared, info_size) = (le32(10)?, le32(14)?); let headers_end = info_size.checked_add(14)?; - if declared >= headers_end { + // At the end of the headers is right only when no masks or colour table follow. + if declared > headers_end { return None; } let (bpp, masks, used, entry) = if info_size == 12 { @@ -93,7 +94,7 @@ fn bmp_pixel_offset_repair(data: &[u8]) -> Option { (n, _) => n, }; let offset = entries.checked_mul(entry)?.checked_add(masks)?.checked_add(headers_end)?; - (usize::try_from(offset).ok()? <= data.len()).then_some(offset) + (offset != declared && usize::try_from(offset).ok()? <= data.len()).then_some(offset) } /// The pixel size, from the header alone; `None` when it can't be read or the image is bigger diff --git a/crates/xfa/src/tests.rs b/crates/xfa/src/tests.rs index 8926c12b2a..8086e36bcc 100644 --- a/crates/xfa/src/tests.rs +++ b/crates/xfa/src/tests.rs @@ -1825,6 +1825,9 @@ fn a_bmp_whose_pixel_offset_points_into_its_headers_is_read_from_after_them() { fields[10..14].copy_from_slice(&0u32.to_le_bytes()); fields.splice(54..54, [0u8; 12]); assert_eq!(bmp_with_pixel_offset(&fields).get(10..14), Some(&66u32.to_le_bytes()[..])); + // Pointing at the end of the 40-byte header, before the masks. + fields[10..14].copy_from_slice(&54u32.to_le_bytes()); + assert_eq!(bmp_with_pixel_offset(&fields).get(10..14), Some(&66u32.to_le_bytes()[..])); // A colour count no file could hold: left alone. let mut huge = tiny_bmp(true); huge[46..50].copy_from_slice(&u32::MAX.to_le_bytes()); @@ -1840,6 +1843,9 @@ fn a_bmp_whose_pixel_offset_points_into_its_headers_is_read_from_after_them() { } core.extend_from_slice(&[0, 0, 255, 255, 0, 0, 0b0100_0000, 0, 0, 0]); assert_eq!(bmp_with_pixel_offset(&core).get(10..14), Some(&32u32.to_le_bytes()[..])); + let mut at_table = core.clone(); + at_table[10..14].copy_from_slice(&26u32.to_le_bytes()); + assert_eq!(bmp_with_pixel_offset(&at_table).get(10..14), Some(&32u32.to_le_bytes()[..]), "before the colour table"); let d = decode(&bmp_with_pixel_offset(&core)).unwrap(); assert_eq!(d.samples, vec![255, 0, 0, 0, 0, 255], "core header"); }