Integrate KenM76 PR #4943

Cargo.lock

@@ -4338,6 +4338,7 @@ dependencies = [
  "pdfcraft-cos",
  "pdfcraft-fonts",
  "pdfcraft-model",
+ "pdfcraft-xfa",
  "png 0.18.1",
  "thiserror 2.0.21",
  "tiff",

crates/create/Cargo.toml

@@ -12,6 +12,8 @@ workspace = true
 pdfcraft-cos = { workspace = true }
 pdfcraft-model = { workspace = true }
 pdfcraft-fonts = { workspace = true }
+# For `image::bmp_with_pixel_offset`, shared with XFA pictures.
+pdfcraft-xfa = { workspace = true }
 png = "0.18"
 tiff = "0.11"
 image = { version = "0.25", default-features = false, features = ["bmp", "gif", "jpeg"] }

crates/create/src/lib.rs

@@ -562,7 +562,7 @@ fn embed(name: &str, bytes: &[u8], wanted: usize) -> Result<Vec<Embedded>, Creat
     } else if is_tiff(bytes) {
         tiff_pages(name, bytes, wanted)
     } else if bytes.starts_with(b"BM") {
-        Ok(vec![decoded(name, bytes, image::ImageFormat::Bmp)?])
+        Ok(vec![decoded(name, &pdfcraft_xfa::image::bmp_with_pixel_offset(bytes), image::ImageFormat::Bmp)?])
     } else if is_gif(bytes) {
         Ok(vec![decoded(name, bytes, image::ImageFormat::Gif)?])
     } else if bytes.starts_with(JP2_SIGNATURE) || bytes.starts_with(&[0xFF, 0x4F, 0xFF, 0x51]) {

crates/create/src/tests.rs

@@ -206,6 +206,26 @@ fn bmp_gif_and_multi_page_tiff_images() {
     assert!(matches!(from_images(&[("x.webp".into(), b"RIFF0000WEBP".to_vec())]), Err(CreateError::Image(..))));
 }
 
+/// #4937: a BMP whose header gives the pixel data's offset as 0 is placed as the same image as
+/// the intact file, not shifted by the header bytes.
+#[test]
+fn a_bmp_with_a_zero_pixel_offset_is_placed_like_the_intact_file() {
+    use image::ImageEncoder;
+    let mut bmp = Vec::new();
+    let pixels: Vec<u8> = (0..18).map(|i| i * 13).collect();
+    image::codecs::bmp::BmpEncoder::new(&mut bmp).write_image(&pixels, 3, 2, image::ExtendedColorType::Rgb8).unwrap();
+    let mut zero = bmp.clone();
+    zero[10..14].copy_from_slice(&0u32.to_le_bytes());
+    let samples = |data: Vec<u8>| {
+        let doc = reopen(&from_images(&[("a.bmp".into(), data)]).unwrap());
+        let res = doc.resolve(pages(&doc)[0].get(b"Resources").unwrap()).as_dict().cloned().unwrap();
+        let xo = doc.resolve(res.get(b"XObject").unwrap()).as_dict().cloned().unwrap();
+        let Object::Stream(s) = &*doc.resolve(xo.get(b"Im0").unwrap()) else { panic!("not an image") };
+        s.decoded().unwrap()
+    };
+    assert_eq!(samples(zero), samples(bmp));
+}
+
 /// #665 lifted the tiff crate's 256 MiB decode limit; a few bytes claiming a 60000x60000 page
 /// (3.6 GB of gray pixels) must still be refused, not allocated.
 #[test]

crates/xfa/src/image.rs

@@ -41,6 +41,62 @@ fn is_bmp(data: &[u8]) -> bool {
         && data.get(14..18).is_some_and(|b| matches!(u32::from_le_bytes([b[0], b[1], b[2], b[3]]), 12 | 16 | 40 | 52 | 56 | 64 | 108 | 124))
 }
 
+/// `data` with a BMP's pixel-data offset (`bfOffBits`) corrected when it points into the file or
+/// 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.
+pub fn bmp_with_pixel_offset(data: &[u8]) -> std::borrow::Cow<'_, [u8]> {
+    match bmp_pixel_offset_repair(data) {
+        Some(offset) => {
+            let mut fixed = data.to_vec();
+            if let Some(field) = fixed.get_mut(10..14) {
+                field.copy_from_slice(&offset.to_le_bytes());
+            }
+            std::borrow::Cow::Owned(fixed)
+        }
+        None => std::borrow::Cow::Borrowed(data),
+    }
+}
+
+/// Where a BMP's rows start when its declared offset lies inside its headers, else `None`.
+fn bmp_pixel_offset_repair(data: &[u8]) -> Option<u32> {
+    let le16 = |at: usize| data.get(at..at + 2).map(|b| u32::from(u16::from_le_bytes([b[0], b[1]])));
+    let le32 = |at: usize| data.get(at..at + 4).map(|b| u32::from_le_bytes([b[0], b[1], b[2], b[3]]));
+    if !is_bmp(data) {
+        return None;
+    }
+    let (declared, info_size) = (le32(10)?, le32(14)?);
+    let headers_end = info_size.checked_add(14)?;
+    // 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 {
+        // BITMAPCOREHEADER: 16-bit fields, no colour count, 3-byte table entries.
+        (le16(24)?, 0, 0, 3)
+    } else if info_size < 40 {
+        // OS/2's 16-byte header ends before the colour count; leave it to the decoder.
+        return None;
+    } else {
+        // BI_BITFIELDS and BI_ALPHABITFIELDS put their masks after a 40-byte header; the larger
+        // headers hold them inside.
+        let masks = match (info_size, le32(30)?) {
+            (40, 3) => 12,
+            (40, 6) => 16,
+            _ => 0,
+        };
+        (le16(28)?, masks, le32(46)?, 4)
+    };
+    let entries = match (used, bpp) {
+        (0, 1..=8) => 1 << bpp,
+        (0, _) => 0,
+        (n, _) => n,
+    };
+    let offset = entries.checked_mul(entry)?.checked_add(masks)?.checked_add(headers_end)?;
+    (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
 /// than can be decoded (so layout gives such a picture its box, not a page-high strip).
 pub fn size(data: &[u8]) -> Option<(u32, u32)> {
@@ -70,7 +126,7 @@ fn header_size(data: &[u8]) -> Option<(u32, u32)> {
             (w > 0 && h > 0).then_some((w, h))
         }
         // The decoders read only the headers (TIFF: the first IFD) until asked for pixels.
-        Kind::Bmp => reader(data, image::ImageFormat::Bmp).into_dimensions().ok().filter(|&(w, h)| w > 0 && h > 0),
+        Kind::Bmp => reader(&bmp_with_pixel_offset(data), image::ImageFormat::Bmp).into_dimensions().ok().filter(|&(w, h)| w > 0 && h > 0),
         Kind::Tiff => {
             use image::{ImageDecoder, metadata::Orientation};
             let mut decoder = reader(data, image::ImageFormat::Tiff).into_decoder().ok()?;
@@ -174,7 +230,8 @@ pub fn decode(data: &[u8]) -> Result<Decoded, String> {
                 Kind::Bmp => image::ImageFormat::Bmp,
                 _ => image::ImageFormat::Tiff,
             };
-            let img = if k == Kind::Tiff { decode_tiff(data) } else { reader(data, format).decode() }
+            let data = if k == Kind::Bmp { bmp_with_pixel_offset(data) } else { std::borrow::Cow::Borrowed(data) };
+            let img = if k == Kind::Tiff { decode_tiff(&data) } else { reader(&data, format).decode() }
                 .map_err(|e| if matches!(e, image::ImageError::Limits(_)) { too_big() } else { bad(e.to_string()) })?;
             let rgba = img.into_rgba8();
             let (width, height) = rgba.dimensions();

crates/xfa/src/tests.rs

@@ -1801,6 +1801,62 @@ fn pictures_are_embedded_once_across_relayouts_and_bad_data_pictures_are_reporte
     assert!(find_image(&t.root.children).is_some_and(|n| n > 1_200_000));
 }
 
+/// #4937: LiveCycle wrote BMPs whose header gives the pixel data's offset as 0. Read literally,
+/// the pixels start at the file header, so the image is drawn shifted and wrapped around.
+#[test]
+fn a_bmp_whose_pixel_offset_points_into_its_headers_is_read_from_after_them() {
+    use crate::image::{bmp_with_pixel_offset, decode, size};
+    for palette in [false, true] {
+        let intact = tiny_bmp(palette);
+        assert!(matches!(bmp_with_pixel_offset(&intact), std::borrow::Cow::Borrowed(_)), "an intact file is not copied");
+        for bad in [0u32, 14, 53] {
+            let mut bmp = intact.clone();
+            bmp[10..14].copy_from_slice(&bad.to_le_bytes());
+            let d = decode(&bmp).unwrap();
+            assert_eq!(d.samples, vec![255, 0, 0, 0, 0, 255], "offset {bad}, palette: {palette}");
+            assert_eq!(size(&bmp), Some((2, 1)));
+            assert_eq!(bmp_with_pixel_offset(&bmp).as_ref(), &intact[..], "offset {bad}, palette: {palette}");
+        }
+    }
+    // Too short to hold the pixels after the headers: left alone, not grown.
+    let mut cut = tiny_bmp(true);
+    cut[10..14].copy_from_slice(&0u32.to_le_bytes());
+    cut.truncate(60);
+    assert_eq!(bmp_with_pixel_offset(&cut).as_ref(), &cut[..]);
+    // A truncated header, and bytes that are not a BMP: left alone.
+    assert_eq!(bmp_with_pixel_offset(b"BM").as_ref(), b"BM");
+    assert_eq!(bmp_with_pixel_offset(TINY_GIF).as_ref(), TINY_GIF);
+    // BI_BITFIELDS in a 40-byte header: three masks follow the header.
+    let mut fields = tiny_bmp(false);
+    fields[30..34].copy_from_slice(&3u32.to_le_bytes());
+    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());
+    huge[10..14].copy_from_slice(&0u32.to_le_bytes());
+    assert_eq!(bmp_with_pixel_offset(&huge).as_ref(), &huge[..]);
+    // A 12-byte core header: 16-bit fields and 3-byte palette entries, `1 << bpp` of them.
+    let mut core = b"BM".to_vec();
+    core.extend_from_slice(&[0; 8]);
+    core.extend_from_slice(&0u32.to_le_bytes());
+    core.extend_from_slice(&12u32.to_le_bytes());
+    for v in [2u16, 1, 1, 1] {
+        core.extend_from_slice(&v.to_le_bytes());
+    }
+    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");
+}
+
 /// Designer forms often carry BMP and TIFF pictures, declared as `image/bmp` and `image/tif`
 /// (the Government of Canada's IMM 5257 and IMM 5710 have both). They were left out.
 #[test]