From 38c948219119510c456a1104089a2b52fa365924 Mon Sep 17 00:00:00 2001 From: KenM76 Date: Tue, 27 Oct 2026 05:00:44 -0400 Subject: [PATCH 1/3] fix(xfa, create): read a BMP's pixels from after its headers when its offset points into them LiveCycle Designer wrote BMPs whose file header gives bfOffBits = 0. image (0.25) seeks to that offset as given, so the pixels were read from the file header on and the picture was drawn shifted and wrapped around (#4937). `pdfcraft_xfa::image::bmp_with_pixel_offset` recomputes the offset from the info header (its size, BI_BITFIELDS masks in a 40-byte header, the palette) when the declared one lies inside the headers and the computed one fits in the file; anything else is passed through unchanged and not copied. XFA pictures (size and decode) and `create::from_images` both go through it, so pdfcraft-create gains a dependency on pdfcraft-xfa (L4 -> L3). Fixes #4937 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SqmHYx8FiWM5PcsoiErgp9 --- Cargo.lock | 1 + crates/create/Cargo.toml | 2 ++ crates/create/src/lib.rs | 2 +- crates/create/src/tests.rs | 20 +++++++++++++ crates/xfa/src/image.rs | 57 ++++++++++++++++++++++++++++++++++++-- crates/xfa/src/tests.rs | 33 ++++++++++++++++++++++ 6 files changed, 112 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3c713ba5cc..2a8bb6ab7d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4187,6 +4187,7 @@ dependencies = [ "pdfcraft-cos", "pdfcraft-fonts", "pdfcraft-model", + "pdfcraft-xfa", "png 0.18.1", "thiserror 2.0.21", "tiff", diff --git a/crates/create/Cargo.toml b/crates/create/Cargo.toml index fa649de47d..12daad621c 100644 --- a/crates/create/Cargo.toml +++ b/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"] } diff --git a/crates/create/src/lib.rs b/crates/create/src/lib.rs index 02bccd5fa8..a6fb28b3d6 100644 --- a/crates/create/src/lib.rs +++ b/crates/create/src/lib.rs @@ -562,7 +562,7 @@ fn embed(name: &str, bytes: &[u8], wanted: usize) -> Result, 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]) { diff --git a/crates/create/src/tests.rs b/crates/create/src/tests.rs index 3c9aa42a0e..5cd34a0f1a 100644 --- a/crates/create/src/tests.rs +++ b/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 = (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| { + 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] diff --git a/crates/xfa/src/image.rs b/crates/xfa/src/image.rs index 5ec42873ed..8f1953bedd 100644 --- a/crates/xfa/src/image.rs +++ b/crates/xfa/src/image.rs @@ -41,6 +41,58 @@ 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, 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 { + 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)?; + 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 { + // 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)?; + (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 +122,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 +226,8 @@ pub fn decode(data: &[u8]) -> Result { 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(); diff --git a/crates/xfa/src/tests.rs b/crates/xfa/src/tests.rs index 39a1c93483..f8c0c6dd78 100644 --- a/crates/xfa/src/tests.rs +++ b/crates/xfa/src/tests.rs @@ -1794,6 +1794,39 @@ 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()[..])); +} + /// 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] From de9ab5370e93ef1e274bb175aa46585b6d99a9ae Mon Sep 17 00:00:00 2001 From: KenM76 Date: Tue, 27 Oct 2026 05:03:47 -0400 Subject: [PATCH 2/3] fix(xfa): leave a 16-byte BMP header to the decoder; test core headers and a huge colour count Review follow-up: the repair reads the 40-byte header's compression and colour count, which a 16-byte OS/2 header does not have. New cases pin a 12-byte core header (3-byte palette entries) and a biClrUsed of u32::MAX. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SqmHYx8FiWM5PcsoiErgp9 --- crates/xfa/src/image.rs | 3 +++ crates/xfa/src/tests.rs | 17 +++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/crates/xfa/src/image.rs b/crates/xfa/src/image.rs index 8f1953bedd..a284b444a8 100644 --- a/crates/xfa/src/image.rs +++ b/crates/xfa/src/image.rs @@ -74,6 +74,9 @@ fn bmp_pixel_offset_repair(data: &[u8]) -> Option { 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. diff --git a/crates/xfa/src/tests.rs b/crates/xfa/src/tests.rs index f8c0c6dd78..8926c12b2a 100644 --- a/crates/xfa/src/tests.rs +++ b/crates/xfa/src/tests.rs @@ -1825,6 +1825,23 @@ 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()[..])); + // 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 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` From fd8c8c185d38169980c00d517eceac8d65601ed3 Mon Sep 17 00:00:00 2001 From: KenM76 Date: Tue, 27 Oct 2026 05:43:31 -0400 Subject: [PATCH 3/3] 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"); }