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