From 41e8824c467b27db7513038064f22254d6d5506b Mon Sep 17 00:00:00 2001 From: ashokram-si <347905169+ashokram-si@users.noreply.github.com> Date: Mon, 26 Oct 2026 08:53:07 +0530 Subject: [PATCH] Metafile import: a file's bitmaps share one decoded-pixel budget (#3600) Each embedded DIB was bounded on its own (2^26 pixels), but not their total, so a small EMF or WMF of many BI_PNG/BI_JPEG records could cost minutes of decoding. The importer now gives a file 2^28 pixels for all its bitmaps, charged from each bitmap's header before it is decoded; bitmaps past it are left out with a warning. --- crates/metafile/src/dib.rs | 85 ++++++++++++++++++----------- crates/metafile/src/import.rs | 7 ++- crates/metafile/src/import/emf.rs | 11 ++-- crates/metafile/src/import/wmf.rs | 2 +- crates/metafile/src/tests.rs | 24 ++++++++ crates/metafile/src/tests_import.rs | 22 ++++++++ 6 files changed, 113 insertions(+), 38 deletions(-) diff --git a/crates/metafile/src/dib.rs b/crates/metafile/src/dib.rs index a091087da..fefd8a200 100644 --- a/crates/metafile/src/dib.rs +++ b/crates/metafile/src/dib.rs @@ -13,6 +13,11 @@ const MAX_PIXELS: u64 = 1 << 24; /// Most pixels along a side, and in all, of a bitmap the reader decodes. const MAX_READ_SIDE: u32 = 1 << 15; const MAX_READ_PIXELS: u64 = 1 << 26; +/// Most pixels the bitmaps of one file may decode to together (four of the largest one), so a small +/// file of many image records can't cost minutes of decoding (#3600). +pub(crate) const MAX_FILE_PIXELS: u64 = 1 << 28; +/// What [`decode`] says of a bitmap past what is left of the file's budget. +pub(crate) const OVER_BUDGET: &str = "the bitmaps hold more pixels than one file may"; const BI_RGB: u32 = 0; const BI_BITFIELDS: u32 = 3; @@ -136,8 +141,16 @@ fn channel(v: u32, mask: u32) -> u8 { ((x * 255 + max / 2) / max.max(1)).min(255) as u8 } -/// Read a DIB from its BITMAPINFO (`bmi`: the header and colour table) and pixel `bits`. -pub(crate) fn decode(bmi: &[u8], bits: &[u8], alpha: Alpha) -> Result { +/// Take a bitmap of `w` × `h` from what is `left` of the file's budget, before it is decoded. +fn charge(w: u32, h: u32, left: &mut u64) -> Result<(), String> { + let n = u64::from(w) * u64::from(h); + *left = left.checked_sub(n).ok_or(OVER_BUDGET)?; + Ok(()) +} + +/// Read a DIB from its BITMAPINFO (`bmi`: the header and colour table) and pixel `bits`, taking its +/// pixels from what is `left` of the file's budget ([`MAX_FILE_PIXELS`]). +pub(crate) fn decode(bmi: &[u8], bits: &[u8], alpha: Alpha, left: &mut u64) -> Result { let bad = || "a bitmap is damaged".to_string(); let mut r = Reader::new(bmi); let size = r.u32().ok_or_else(bad)?; @@ -166,6 +179,7 @@ pub(crate) fn decode(bmi: &[u8], bits: &[u8], alpha: Alpha) -> Result MAX_READ_SIDE || h > MAX_READ_SIDE || u64::from(w) * u64::from(h) > MAX_READ_PIXELS { return Err("a bitmap is too large to read".into()); } + charge(w, h, left)?; let img = image::load_from_memory(bits).map_err(|_| bad())?.to_rgba8(); return Ok(Rgba { width: w, height: h, pixels: img.into_raw() }); } @@ -210,6 +224,7 @@ pub(crate) fn decode(bmi: &[u8], bits: &[u8], alpha: Alpha) -> Result (r, g, b, a), (None, 16) => (0x7c00, 0x03e0, 0x001f, 0), @@ -288,34 +303,42 @@ mod tests { Rgba { width: 3, height: 2, pixels } } - /// CRC-32 of a PNG chunk. - fn crc32(data: &[u8]) -> u32 { - let mut crc = !0u32; - for &b in data { - crc ^= u32::from(b); - for _ in 0..8 { - crc = if crc & 1 == 1 { (crc >> 1) ^ 0xEDB8_8320 } else { crc >> 1 }; - } - } - !crc + /// A whole file's budget. + fn budget() -> u64 { + MAX_FILE_PIXELS + } + + /// A BITMAPINFOHEADER of an embedded PNG. + fn png_header() -> Vec { + let mut bmi = vec![0u8; 40]; + bmi[..4].copy_from_slice(&40u32.to_le_bytes()); + bmi[16..20].copy_from_slice(&BI_PNG.to_le_bytes()); + bmi } #[test] fn oversized_png_is_refused_from_its_header() { - let mut png = vec![]; - image::GrayImage::new(1, 1).write_to(&mut Cursor::new(&mut png), image::ImageFormat::Png).unwrap(); - // Claim a width over the limit in the IHDR chunk (and fix its CRC); the pixel data no longer - // matches, so decoding it would fail with a different error. - png[16..20].copy_from_slice(&(MAX_READ_SIDE + 1).to_be_bytes()); - let crc = crc32(&png[12..29]); - png[29..33].copy_from_slice(&crc.to_be_bytes()); - let mut bmi = vec![0u8; 40]; - bmi[..4].copy_from_slice(&40u32.to_le_bytes()); - bmi[16..20].copy_from_slice(&BI_PNG.to_le_bytes()); - let err = decode(&bmi, &png, Alpha::Ignore).unwrap_err(); + // A width over the limit in the IHDR chunk: decoding the pixels would fail with a different error. + let png = crate::tests::png_claiming(MAX_READ_SIDE + 1, 1); + let err = decode(&png_header(), &png, Alpha::Ignore, &mut budget()).unwrap_err(); assert_eq!(err, "a bitmap is too large to read"); } + #[test] + fn bitmaps_share_the_file_budget() { + // #3600: each bitmap is charged before it is decoded, and one past what is left is refused. + let (bmi, bits) = rgb24(&sample(), 1.0); + let mut left = 6 + 5; + assert!(decode(&bmi, &bits, Alpha::Ignore, &mut left).is_ok()); + assert_eq!(left, 5); + assert_eq!(decode(&bmi, &bits, Alpha::Ignore, &mut left).unwrap_err(), OVER_BUDGET); + assert_eq!(left, 5, "a refused bitmap costs nothing"); + // An embedded PNG is charged from its header, so one past the budget is never decoded. + let mut left = (1 << 20) - 1; + let png = crate::tests::png_claiming(1 << 10, 1 << 10); + assert_eq!(decode(&png_header(), &png, Alpha::Ignore, &mut left).unwrap_err(), OVER_BUDGET); + } + #[test] fn rgb24_is_bottom_up_bgr_over_white() { let (bmi, bits) = rgb24(&sample(), 1.0); @@ -343,7 +366,7 @@ mod tests { assert_eq!(bmi.len(), 40); // Rows of 9 bytes padded to 12. assert_eq!(bits.len(), 24); - let back = decode(&bmi, &bits, Alpha::Ignore).unwrap(); + let back = decode(&bmi, &bits, Alpha::Ignore, &mut budget()).unwrap(); assert_eq!((back.width, back.height), (3, 2)); assert_eq!(&back.pixels[..12], &[255, 0, 0, 255, 0, 255, 0, 255, 0, 0, 255, 255]); // Half-transparent black over white is mid grey; transparent is white. @@ -354,12 +377,12 @@ mod tests { #[test] fn bgra32_round_trips_with_alpha() { let (bmi, bits) = bgra32(&sample(), 1.0); - let back = decode(&bmi, &bits, Alpha::Premultiplied).unwrap(); + let back = decode(&bmi, &bits, Alpha::Premultiplied, &mut budget()).unwrap(); assert_eq!(&back.pixels[..12], &sample().pixels[..12]); assert_eq!(back.pixels[15], 128); assert_eq!(back.pixels[19], 0); // Read as StretchDIBits does, alpha is ignored. - assert!(decode(&bmi, &bits, Alpha::Ignore).unwrap().opaque()); + assert!(decode(&bmi, &bits, Alpha::Ignore, &mut budget()).unwrap().opaque()); } #[test] @@ -367,13 +390,13 @@ mod tests { // A 1-bit 2 × 1 bitmap: black then white. let mut bmi = header(2, 1, 1, 4); bmi.extend_from_slice(&[0, 0, 0, 0, 255, 255, 255, 0]); - let back = decode(&bmi, &[0b0100_0000, 0, 0, 0], Alpha::Ignore).unwrap(); + let back = decode(&bmi, &[0b0100_0000, 0, 0, 0], Alpha::Ignore, &mut budget()).unwrap(); assert_eq!(back.pixels, vec![0, 0, 0, 255, 255, 255, 255, 255]); // Too few bits, a silly size, an unknown depth: errors, no panic. - assert!(decode(&bmi, &[0], Alpha::Ignore).is_err()); - assert!(decode(&header(1 << 20, 1 << 20, 24, 0), &[], Alpha::Ignore).is_err()); - assert!(decode(&header(1, 1, 7, 0), &[0; 4], Alpha::Ignore).is_err()); - assert!(decode(&[1, 2], &[], Alpha::Ignore).is_err()); + assert!(decode(&bmi, &[0], Alpha::Ignore, &mut budget()).is_err()); + assert!(decode(&header(1 << 20, 1 << 20, 24, 0), &[], Alpha::Ignore, &mut budget()).is_err()); + assert!(decode(&header(1, 1, 7, 0), &[0; 4], Alpha::Ignore, &mut budget()).is_err()); + assert!(decode(&[1, 2], &[], Alpha::Ignore, &mut budget()).is_err()); } #[test] diff --git a/crates/metafile/src/import.rs b/crates/metafile/src/import.rs index c56293022..ed31d496e 100644 --- a/crates/metafile/src/import.rs +++ b/crates/metafile/src/import.rs @@ -20,7 +20,7 @@ use vectorcraft_doc::{ }; use vectorcraft_geom::{Affine, BezPath, FillRule, PathData, PathEl, Point, Rect, Shape, Vec2, shapes}; -use crate::dib::Rgba; +use crate::dib::{self, Rgba}; use crate::{Imported, Kind}; /// Most objects an import makes (a hostile file can't exhaust memory with them). @@ -47,6 +47,7 @@ const HATCHES: &str = "hatched brushes are filled with their colour"; const PATTERN_BRUSHES: &str = "pattern brushes are filled grey"; const EXCLUDED_CLIPS: &str = "clip areas cut out of the clip (excluded rectangles, differences) are ignored"; const BAD_IMAGE: &str = "an image that couldn't be read was left out"; +const TOO_MANY_PIXELS: &str = "images past the most pixels one file may hold were left out"; const FAR_AWAY: &str = "objects far outside the picture were left out"; /// Is `r` finite and within [`LIMIT`]? @@ -196,6 +197,8 @@ pub(crate) struct Player { warnings: Vec, skipped: usize, next_clip: u32, + /// Pixels the file's bitmaps may still decode to ([`dib::MAX_FILE_PIXELS`]). + pub pixels_left: u64, /// The file holds EMF+ records (which aren't read). pub emf_plus: bool, } @@ -280,6 +283,7 @@ impl Player { warnings: vec![], skipped: 0, next_clip: 0, + pixels_left: dib::MAX_FILE_PIXELS, emf_plus: false, } } @@ -773,6 +777,7 @@ impl Player { } let img = match img { Ok(i) => i, + Err(e) if e == dib::OVER_BUDGET => return self.warn(TOO_MANY_PIXELS), Err(_) => return self.warn(BAD_IMAGE), }; let img = crop(img, src); diff --git a/crates/metafile/src/import/emf.rs b/crates/metafile/src/import/emf.rs index e2024af22..2b97a5a37 100644 --- a/crates/metafile/src/import/emf.rs +++ b/crates/metafile/src/import/emf.rs @@ -78,10 +78,10 @@ fn font(r: &mut Reader) -> Option { } /// The bitmap of an image record: its BITMAPINFO and bits at the offsets the record gives. -fn bitmap(rec: &[u8], off_bmi: u32, cb_bmi: u32, off_bits: u32, cb_bits: u32, alpha: Alpha) -> Result { +fn bitmap(rec: &[u8], off_bmi: u32, cb_bmi: u32, off_bits: u32, cb_bits: u32, alpha: Alpha, left: &mut u64) -> Result { let bmi = slice(rec, off_bmi, cb_bmi).ok_or("damaged")?; let bits = slice(rec, off_bits, cb_bits).ok_or("damaged")?; - dib::decode(bmi, bits, alpha) + dib::decode(bmi, bits, alpha, left) } /// The rows a DIB's source rectangle counts from the bottom in (StretchDIBits on a bottom-up DIB). @@ -286,7 +286,7 @@ fn record(p: &mut Player, kind: u32, rec: &[u8]) -> Option<()> { if cb_bmi == 0 { p.pattern_blit(d, rop); } else { - let img = bitmap(rec, off_bmi, cb_bmi, off_bits, cb_bits, Alpha::Ignore); + let img = bitmap(rec, off_bmi, cb_bmi, off_bits, cb_bits, Alpha::Ignore, &mut p.pixels_left); let src = source(xs, ys, ws, hs, &img, false); p.image(img, src, d, rop, 1.0); } @@ -298,7 +298,7 @@ fn record(p: &mut Player, kind: u32, rec: &[u8]) -> Option<()> { r.skip(4)?; let rop = r.u32()?; let (wd, hd) = (r.i32()?, r.i32()?); - let img = bitmap(rec, off_bmi, cb_bmi, off_bits, cb_bits, Alpha::Ignore); + let img = bitmap(rec, off_bmi, cb_bmi, off_bits, cb_bits, Alpha::Ignore, &mut p.pixels_left); let src = source(xs, ys, ws, hs, &img, bottom_up(rec, off_bmi)); let d = Rect::new(f64::from(xd), f64::from(yd), f64::from(xd) + f64::from(wd), f64::from(yd) + f64::from(hd)); p.image(img, src, d, rop, 1.0); @@ -344,7 +344,8 @@ fn record(p: &mut Player, kind: u32, rec: &[u8]) -> Option<()> { r.skip(24 + 4 + 4)?; let (off_bmi, cb_bmi, off_bits, cb_bits) = (r.u32()?, r.u32()?, r.u32()?, r.u32()?); let (ws, hs) = (r.i32()?, r.i32()?); - let img = bitmap(rec, off_bmi, cb_bmi, off_bits, cb_bits, if per_pixel { Alpha::Premultiplied } else { Alpha::Ignore }); + let alpha = if per_pixel { Alpha::Premultiplied } else { Alpha::Ignore }; + let img = bitmap(rec, off_bmi, cb_bmi, off_bits, cb_bits, alpha, &mut p.pixels_left); let src = source(xs, ys, ws, hs, &img, false); let d = Rect::new(f64::from(xd), f64::from(yd), f64::from(xd) + f64::from(wd), f64::from(yd) + f64::from(hd)); p.image(img, src, d, crate::import::SRCCOPY, f64::from(constant) / 255.0); diff --git a/crates/metafile/src/import/wmf.rs b/crates/metafile/src/import/wmf.rs index ec99aadc9..d0fbd190d 100644 --- a/crates/metafile/src/import/wmf.rs +++ b/crates/metafile/src/import/wmf.rs @@ -166,7 +166,7 @@ fn blit(p: &mut Player, rop: u32, src: (i16, i16, i16, i16), dest: (i16, i16, i1 let d = Rect::new(xd, yd, xd + wd, yd + hd); let Some(packed) = packed else { return p.pattern_blit(d, rop) }; let parts = dib::split(packed); - let img = parts.ok_or_else(|| "damaged".to_string()).and_then(|(bmi, bits)| dib::decode(bmi, bits, Alpha::Ignore)); + let img = parts.ok_or_else(|| "damaged".to_string()).and_then(|(bmi, bits)| dib::decode(bmi, bits, Alpha::Ignore, &mut p.pixels_left)); let bottom_up = parts.and_then(|(bmi, _)| bmi.get(8..12)).is_some_and(|h| i32::from_le_bytes([h[0], h[1], h[2], h[3]]) > 0); let ih = img.as_ref().map_or(0.0, |i| f64::from(i.height)); let (xs, ys, ws, hs) = (f64::from(src.0), f64::from(src.1), f64::from(src.2), f64::from(src.3)); diff --git a/crates/metafile/src/tests.rs b/crates/metafile/src/tests.rs index c2fb4276a..20612545a 100644 --- a/crates/metafile/src/tests.rs +++ b/crates/metafile/src/tests.rs @@ -46,6 +46,30 @@ pub(crate) fn png(w: u32, h: u32, rgba: [u8; 4]) -> Vec { out } +/// CRC-32 of a PNG chunk. +pub(crate) fn crc32(data: &[u8]) -> u32 { + let mut crc = !0u32; + for &b in data { + crc ^= u32::from(b); + for _ in 0..8 { + crc = if crc & 1 == 1 { (crc >> 1) ^ 0xEDB8_8320 } else { crc >> 1 }; + } + } + !crc +} + +/// A 1 × 1 PNG whose IHDR claims `w` × `h` (and whose CRC matches): its size reads from the header, +/// but its pixel data no longer matches, so decoding it fails. +pub(crate) fn png_claiming(w: u32, h: u32) -> Vec { + let mut png = vec![]; + image::GrayImage::new(1, 1).write_to(&mut std::io::Cursor::new(&mut png), image::ImageFormat::Png).unwrap(); + png[16..20].copy_from_slice(&w.to_be_bytes()); + png[20..24].copy_from_slice(&h.to_be_bytes()); + let crc = crc32(&png[12..29]); + png[29..33].copy_from_slice(&crc.to_be_bytes()); + png +} + /// An image node of `bytes` placed by `xf`, its blob added to `d`. pub(crate) fn image_node(d: &mut Document, bytes: Vec, w: u32, h: u32, xf: Affine) -> Node { let blob = ImageBlob::new("image/png", bytes); diff --git a/crates/metafile/src/tests_import.rs b/crates/metafile/src/tests_import.rs index f145a11d3..83d2b8db2 100644 --- a/crates/metafile/src/tests_import.rs +++ b/crates/metafile/src/tests_import.rs @@ -329,6 +329,28 @@ fn damaged_files_fail_or_import_partly() { assert!(import(&huge).is_ok()); } +/// An EMR_STRETCHDIBITS copying `png` (a BI_PNG bitmap) to a 10 × 10 box. +fn png_stretch_dibits(png: &[u8]) -> Vec { + // Bounds; destination, source and its size; the BITMAPINFO and the bits (from the record start); + // usage, SRCCOPY and the destination size; then the BITMAPINFOHEADER and the PNG. + let mut body = vec![0; 16]; + body.extend(le(&[0, 0, 0, 0, 1, 1])); + body.extend(le(&[80, 40, 120, png.len() as i32])); + body.extend(le(&[0, 0x00CC_0020, 10, 10])); + body.extend(le(&[40, 1, 1, 1, 5, 0, 0, 0, 0, 0])); + body.extend(png); + body +} + +#[test] +fn embedded_bitmaps_share_one_pixel_budget() { + // #3600: each bitmap was bounded but not their total, so many records of the largest one a record + // may hold cost minutes. Those past the file's budget are left out before they're decoded. + let records = vec![(81, png_stretch_dibits(&png_claiming(1 << 13, 1 << 13))); 5]; + let back = import(&hand_emf(5000, 3000, &records)).unwrap(); + assert!(back.warnings.iter().any(|w| w == "images past the most pixels one file may hold were left out"), "{:?}", back.warnings); +} + #[test] fn clips_nest_only_so_deep() { // Each clip inside the last one used to nest a clipping group one level deeper, past the stack.