From c55a9f771e4d403d1e31f67b1fa8331a00378335 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 24 Oct 2026 05:06:04 -0400 Subject: [PATCH] security(metafile): harden EMF+ parsing, clipping complexity, and memory accounting (#1280) * fix(metafile): harden EMF+ parsing, clipping complexity, and memory accounting - Row 72: Use checked tail slicing in comment header offset calculations - Row 78: Break FillRects and DrawRects loops early when canvas is full - Row 75: Pre-check dimensions in decode_compressed before RGBA pixel allocation - Row 85: Account for EMF+ object table image/path memory against MAX_BYTES - Row 42: Replace O(N^2) prefix copying in wrap_line with linear measurement - Row 43 & 54: Share immutable clip regions Arc across placed items and charge canvas regions - Row 48: Bound text transformation scale and glyph curve geometry in Picture::place and draw_text - Row 52 & 68: Impose work budget bounds on Sutherland-Hodgman polygon clipping and bitmap masking * fix(metafile): collapse nested if statement per clippy --- crates/metafile/src/canvas.rs | 3 +- crates/metafile/src/cliprect.rs | 10 ++++ crates/metafile/src/emfplus.rs | 99 +++++++++++++++++++++++++++------ crates/metafile/src/place.rs | 49 ++++++++++++---- crates/metafile/src/tests.rs | 24 ++++++++ crates/render/src/metafile.rs | 15 +++-- 6 files changed, 168 insertions(+), 32 deletions(-) diff --git a/crates/metafile/src/canvas.rs b/crates/metafile/src/canvas.rs index f286dc61..c6441bca 100644 --- a/crates/metafile/src/canvas.rs +++ b/crates/metafile/src/canvas.rs @@ -390,6 +390,7 @@ impl Canvas { return; } raw.set_region(self.clip.clone()); + let region_cost = self.clip.as_ref().map_or(0, |c| c.len() * std::mem::size_of::<[f64; 4]>()); let cost = match &raw { Raw::Path { ops, .. } => ops.len() * std::mem::size_of::(), // A tile shared with its brush is counted once (by the brush); one made for this fill @@ -400,7 +401,7 @@ impl Canvas { } Raw::Bitmap { img, .. } => img.px.len(), Raw::Text { text: t, .. } => t.text.len() + t.dx.len() * 4 + t.font.family.len() + 128, - }; + } + region_cost; if self.items.len() >= MAX_ITEMS || self.used.saturating_add(cost) > MAX_BYTES { self.full = true; } else { diff --git a/crates/metafile/src/cliprect.rs b/crates/metafile/src/cliprect.rs index 396f4402..fd9dfc14 100644 --- a/crates/metafile/src/cliprect.rs +++ b/crates/metafile/src/cliprect.rs @@ -100,8 +100,14 @@ fn is_rect(clip: &[P]) -> bool { /// Sutherland–Hodgman: `poly` cut by convex `clip`. pub(crate) fn clip_polygon(poly: &[P], clip: &[P]) -> Vec

{ + if poly.len().saturating_mul(clip.len()) > 1_000_000 { + return Vec::new(); + } let mut poly = poly.to_vec(); for i in 0..clip.len() { + if poly.len().saturating_mul(clip.len() - i) > 1_000_000 { + return Vec::new(); + } let (a, b) = (clip[i], clip[(i + 1) % clip.len()]); let keep = |p: P| cross(a, b, p) >= 0.0; let cut = |p: P, q: P| { @@ -307,6 +313,10 @@ fn crop(rect: [f64; 4], img: Rgba, clip: &[P]) -> Option<([f64; 4], Rgba)> { return None; } let mask = !is_rect(clip); + let total_pixels = (x1 - x0).saturating_mul(y1 - y0); + if mask && total_pixels.saturating_mul(clip.len()) > 5_000_000 { + return None; + } let mut px = Vec::with_capacity((x1 - x0) * (y1 - y0) * 4); for row in y0..y1 { // Rows are top-down unless the image says otherwise. diff --git a/crates/metafile/src/emfplus.rs b/crates/metafile/src/emfplus.rs index 3324f5f9..a4dcd4c2 100644 --- a/crates/metafile/src/emfplus.rs +++ b/crates/metafile/src/emfplus.rs @@ -222,6 +222,7 @@ pub(crate) struct Plus { /// Convex device clip polygon; None is unclipped. clip: Area, objects: Vec>, + object_bytes: usize, stack: Vec, partial: Option, /// The picture frame in device units (`[x, y, w, h]`), what Clear paints. @@ -237,6 +238,7 @@ impl Plus { page: 1.0, clip: None, objects: vec![None; MAX_OBJECTS], + object_bytes: 0, stack: Vec::new(), partial: None, frame: None, @@ -247,14 +249,16 @@ impl Plus { pub fn comment(&mut self, cv: &mut Canvas, data: Bytes) { let mut pos = 0usize; while !cv.full() { - let (Some(typ), Some(flags), Some(size), Some(dsize)) = (data.u16(pos), data.u16(pos + 2), data.u32(pos + 4), data.u32(pos + 8)) else { + let Some(tail) = data.0.get(pos..) else { break }; + let tail = Bytes(tail); + let (Some(typ), Some(flags), Some(size), Some(dsize)) = (tail.u16(0), tail.u16(2), tail.u32(4), tail.u32(8)) else { break; }; let (size, dsize) = (size as usize, dsize as usize); if size < 12 || dsize > size - 12 { break; } - let Some(rec) = data.slice(pos + 12, dsize) else { break }; + let Some(rec) = tail.slice(12, dsize) else { break }; let _ = self.record(cv, typ, flags, Bytes(rec)); let Some(next) = pos.checked_add(size) else { break }; pos = next; @@ -290,6 +294,9 @@ impl Plus { let fill = self.brush_of(flags, r.u32(0)?)?; let p = self.painter(); for rc in rects(r, 8, r.u32(4)? as usize, compressed)? { + if cv.full() { + break; + } p.fill(cv, &rect_ops(rc), &fill); } Some(()) @@ -298,6 +305,9 @@ impl Plus { let pen = self.pen(id)?; let p = self.painter(); for rc in rects(r, 4, r.u32(0)? as usize, compressed)? { + if cv.full() { + break; + } p.stroke(cv, &rect_ops(rc), pen); } Some(()) @@ -550,8 +560,28 @@ impl Plus { } fn store(&mut self, id: usize, obj: Option) { - if let Some(slot) = self.objects.get_mut(id) { - *slot = obj; + let Some(slot) = self.objects.get_mut(id) else { return }; + if let Some(old) = slot.take() { + self.object_bytes = self.object_bytes.saturating_sub(Self::obj_cost(&old)); + } + if let Some(new_obj) = obj { + let cost = Self::obj_cost(&new_obj); + if self.object_bytes.saturating_add(cost) <= crate::MAX_BYTES { + self.object_bytes += cost; + *slot = Some(new_obj); + } + } + } + + fn obj_cost(obj: &Obj) -> usize { + match obj { + Obj::Image(img) => match img.as_ref() { + ImageData::Bitmap(rgba) => rgba.px.len(), + ImageData::Meta(pic) => pic.items.len().saturating_mul(128), + }, + Obj::Brush(Fill::Texture { img, .. }) => img.px.len(), + Obj::Path(ops) => ops.len().saturating_mul(std::mem::size_of::()), + _ => 0, } } @@ -1282,11 +1312,18 @@ fn decode_compressed(bytes: &[u8]) -> Option { limits.max_image_height = Some(1 << 15); limits.max_alloc = Some((crate::MAX_PIXELS * 4) as u64); reader.limits(limits); - let img = reader.decode().ok()?.to_rgba8(); - let (w, h) = (img.width() as usize, img.height() as usize); - if w.checked_mul(h)? > crate::MAX_PIXELS { + let (w, h) = reader.into_dimensions().ok()?; + let (w, h) = (w as usize, h as usize); + if w == 0 || h == 0 || w.checked_mul(h)? > crate::MAX_PIXELS { return None; } + let mut reader = image::ImageReader::new(std::io::Cursor::new(bytes)).with_guessed_format().ok()?; + let mut limits = image::Limits::default(); + limits.max_image_width = Some(1 << 15); + limits.max_image_height = Some(1 << 15); + limits.max_alloc = Some((crate::MAX_PIXELS * 4) as u64); + reader.limits(limits); + let img = reader.decode().ok()?.to_rgba8(); Some(Rgba { w, h, bottom_up: false, px: img.into_raw() }) } @@ -1406,31 +1443,61 @@ fn wrap_line(line: &[u16], width: f64, em: f64) -> Vec> { if em <= 0.0 || !width.is_finite() { return vec![line.to_vec()]; } - let fits = |l: &[u16]| l.iter().map(|u| char_em(*u) * em).sum::() <= width; + let trimmed = |v: &[u16]| v.iter().rposition(|u| *u != 0x20).map_or(0, |i| i + 1); + let measure = |v: &[u16]| v.iter().map(|u| char_em(*u) * em).sum::(); + let mut out: Vec> = Vec::new(); let mut cur: Vec = Vec::new(); + let mut cur_total_w = 0.0; + for word in line.split_inclusive(|u| *u == 0x20) { - let mut cand = cur.clone(); - cand.extend_from_slice(word); - let trimmed = |v: &[u16]| v.iter().rposition(|u| *u != 0x20).map_or(0, |i| i + 1); - if fits(&cand[..trimmed(&cand)]) || cur.is_empty() && fits(&word[..trimmed(word)]) { - cur = cand; + let n_non_space = trimmed(word); + let word_non_space_w = measure(&word[..n_non_space]); + let word_total_w = measure(word); + + if cur.is_empty() { + if word_non_space_w <= width { + cur.extend_from_slice(word); + cur_total_w = word_total_w; + continue; + } + } else if cur_total_w + word_non_space_w <= width { + cur.extend_from_slice(word); + cur_total_w += word_total_w; continue; } + if !cur.is_empty() { let n = trimmed(&cur); cur.truncate(n); out.push(std::mem::take(&mut cur)); } + + if word_non_space_w <= width { + cur.extend_from_slice(word); + cur_total_w = word_total_w; + continue; + } + // A word wider than the line on its own: break it between characters. + let mut cur_w = 0.0; for &u in word { - cur.push(u); - if !fits(&cur[..trimmed(&cur)]) && cur.len() > 1 { - cur.pop(); + let u_w = char_em(u) * em; + if cur.is_empty() { + cur.push(u); + cur_w = u_w; + } else if cur_w + u_w <= width { + cur.push(u); + cur_w += u_w; + } else { + let n = trimmed(&cur); + cur.truncate(n); out.push(std::mem::take(&mut cur)); cur.push(u); + cur_w = u_w; } } + cur_total_w = cur_w; } let n = cur.iter().rposition(|u| *u != 0x20).map_or(0, |i| i + 1); cur.truncate(n); diff --git a/crates/metafile/src/place.rs b/crates/metafile/src/place.rs index d9d3bdf8..be00ea22 100644 --- a/crates/metafile/src/place.rs +++ b/crates/metafile/src/place.rs @@ -1,6 +1,8 @@ //! Placing a picture on a target (a page or a device): the fitted transform, paths as kurbo paths in //! target units and bitmaps as target rectangles. The rasteriser and the PDF writer only paint the result. +use std::sync::Arc; + use kurbo::{Affine, BezPath, Rect, Shape}; use crate::{Item, Picture, Seg}; @@ -11,20 +13,22 @@ pub const MAX_BITMAP_SIDE: u32 = 30_000; const MAX_COORD: f64 = 1e6; /// Smallest placed bitmap side (target units), so the stretch onto it is never singular. const MIN_BITMAP_SIDE: f64 = 1e-6; +/// Maximum aggregate path elements retained in placed output. +const MAX_PLACED_ELEMENTS: usize = 500_000; /// One item of a placed picture, in target units. pub enum PlacedItem { /// A filled and/or stroked path. The stroke width is in target units; 0 is a cosmetic hairline that /// the renderer draws at its minimum width. - Path { path: BezPath, fill: Option<[u8; 4]>, stroke: Option<([u8; 4], f64)>, even_odd: bool, region: Option }, + Path { path: BezPath, fill: Option<[u8; 4]>, stroke: Option<([u8; 4], f64)>, even_odd: bool, region: Option> }, /// `Picture::items[index]`, a bitmap, stretched into `rect`. - Bitmap { index: usize, rect: Rect, region: Option }, + Bitmap { index: usize, rect: Rect, region: Option> }, /// `Picture::items[index]`, a pattern fill: `path` (target units) filled with the item's tile /// repeated; `tile` maps tile pixels to target units. - Pattern { index: usize, path: BezPath, even_odd: bool, tile: Affine, region: Option }, + Pattern { index: usize, path: BezPath, even_odd: bool, tile: Affine, region: Option> }, /// `Picture::items[index]`, a text string: `transform` maps its text space to target units; /// `clip` (target units) bounds what it draws. `region` is the device-context clip. - Text { index: usize, transform: Affine, clip: Option, region: Option }, + Text { index: usize, transform: Affine, clip: Option, region: Option> }, } impl Picture { @@ -53,42 +57,65 @@ impl Picture { let c = tf.as_coeffs(); let k = (c[0].abs() + c[3].abs()) / 2.0; let mut out = Vec::with_capacity(self.items.len()); + let mut total_elements = 0usize; + let mut cached_region: Option<(*const [f32; 4], usize, Option>>)> = None; + let mut get_region = |reg: &Option>| -> Option>> { + let (ptr, len) = match reg { + None => (std::ptr::null(), 0), + Some(v) => (v.as_ptr(), v.len()), + }; + if let Some((c_ptr, c_len, ref res)) = cached_region + && c_ptr == ptr + && c_len == len + { + return res.clone(); + } + let res = placed_region(&tf, reg); + cached_region = Some((ptr, len, res.clone())); + res + }; + for (index, item) in self.items.iter().enumerate() { + if total_elements >= MAX_PLACED_ELEMENTS { + break; + } match item { Item::Path { segs, fill, stroke, even_odd, region } => { let path = tf * bez(segs); - let Some(region) = placed_region(&tf, region) else { continue }; + let Some(region) = get_region(region) else { continue }; if in_range(path.bounding_box()) { + total_elements = total_elements.saturating_add(path.elements().len()); let stroke = stroke.map(|(c, w)| (c, f64::from(w) * k)); out.push(PlacedItem::Path { path, fill: *fill, stroke, even_odd: *even_odd, region }); } } Item::Pattern { segs, even_odd, tile: t, region } => { let path = tf * bez(segs); - let Some(region) = placed_region(&tf, region) else { continue }; + let Some(region) = get_region(region) else { continue }; let tile = tf * Affine::translate((f64::from(t.origin[0]), f64::from(t.origin[1]))) * Affine::scale_non_uniform(f64::from(t.cell[0]), f64::from(t.cell[1])); let det = tile.determinant(); if in_range(path.bounding_box()) && det.is_finite() && det.abs() > 1e-12 { + total_elements = total_elements.saturating_add(path.elements().len()); out.push(PlacedItem::Pattern { index, path, even_odd: *even_odd, tile, region }); } } Item::Text(t) => { let transform = tf * Affine::new(t.transform); - let Some(region) = placed_region(&tf, &t.region) else { continue }; + let Some(region) = get_region(&t.region) else { continue }; let clip = t.clip.map(|[x0, y0, x1, y1]| tf.transform_rect_bbox(Rect::new(f64::from(x0), f64::from(y0), f64::from(x1), f64::from(y1)))); let c = transform.as_coeffs(); let origin = Rect::new(c[4], c[5], c[4], c[5]); - if in_range(origin) && c.iter().all(|v| v.is_finite()) && clip.is_none_or(in_range) { + if in_range(origin) && c.iter().all(|v| v.is_finite() && v.abs() <= MAX_COORD) && clip.is_none_or(in_range) { out.push(PlacedItem::Text { index, transform, clip, region }); } } Item::Bitmap { rect: [x, y, w, h], region, .. } => { let (x0, y0) = (f64::from(*x), f64::from(*y)); let rect = tf.transform_rect_bbox(Rect::new(x0, y0, x0 + f64::from(*w), y0 + f64::from(*h))); - let Some(region) = placed_region(&tf, region) else { continue }; + let Some(region) = get_region(region) else { continue }; if in_range(rect) && rect.width() > MIN_BITMAP_SIDE && rect.height() > MIN_BITMAP_SIDE { out.push(PlacedItem::Bitmap { index, rect, region }); } @@ -113,7 +140,7 @@ impl Item { /// The device-context clip in target units. `Some(None)` is the default clip. `None` means the /// region does not meet the page, so the item draws nothing. -fn placed_region(tf: &Affine, region: &Option>) -> Option> { +fn placed_region(tf: &Affine, region: &Option>) -> Option>> { let Some(rects) = region else { return Some(None) }; let mut path = BezPath::new(); for [x0, y0, x1, y1] in rects { @@ -122,7 +149,7 @@ fn placed_region(tf: &Affine, region: &Option>) -> Option 10, "wrapped into multiple lines: {text_items}"); +} + +#[test] +fn clip_polygon_work_bound() { + let poly: Vec<(f64, f64)> = (0..2000).map(|i| (i as f64, i as f64)).collect(); + let clip: Vec<(f64, f64)> = (0..2000).map(|i| (i as f64, i as f64)).collect(); + let clipped = crate::cliprect::clip_polygon(&poly, &clip); + assert!(clipped.is_empty(), "excessive polygon clip product rejected"); +} diff --git a/crates/render/src/metafile.rs b/crates/render/src/metafile.rs index bb9da83f..91fa0138 100644 --- a/crates/render/src/metafile.rs +++ b/crates/render/src/metafile.rs @@ -51,7 +51,7 @@ pub fn rasterize(bytes: &[u8], side: u32) -> Option { ctx.push_clip_layer(&r.to_path(0.1)); for it in pic.place(r, [0.0; 4]) { match it { - PlacedItem::Path { path, fill, stroke, even_odd, region } => clipped(&mut ctx, region.as_ref(), |ctx| { + PlacedItem::Path { path, fill, stroke, even_odd, region } => clipped(&mut ctx, region.as_deref(), |ctx| { if let Some(f) = fill { ctx.set_paint(rgba(f)); ctx.set_fill_rule(if even_odd { peniko::Fill::EvenOdd } else { peniko::Fill::NonZero }); @@ -64,7 +64,7 @@ pub fn rasterize(bytes: &[u8], side: u32) -> Option { ctx.stroke_path(&path); } }), - PlacedItem::Bitmap { index, rect, region } => clipped(&mut ctx, region.as_ref(), |ctx| { + PlacedItem::Bitmap { index, rect, region } => clipped(&mut ctx, region.as_deref(), |ctx| { let Some(Some(pm)) = pixmaps.get(index) else { return }; let (bw, bh) = (pm.width().max(1) as f64, pm.height().max(1) as f64); ctx.set_paint(vello_cpu::Image { @@ -75,7 +75,7 @@ pub fn rasterize(bytes: &[u8], side: u32) -> Option { ctx.fill_rect(&rect); ctx.reset_paint_transform(); }), - PlacedItem::Pattern { index, path, even_odd, tile, region } => clipped(&mut ctx, region.as_ref(), |ctx| { + PlacedItem::Pattern { index, path, even_odd, tile, region } => clipped(&mut ctx, region.as_deref(), |ctx| { let Some(Item::Pattern { tile: t, .. }) = pic.items.get(index) else { return }; let key = Arc::as_ptr(t); let (pm, avg) = tiles.entry(key).or_insert_with(|| (premul(t.width, t.height, &t.rgba), average(&t.rgba))).clone(); @@ -102,7 +102,7 @@ pub fn rasterize(bytes: &[u8], side: u32) -> Option { } } }), - PlacedItem::Text { index, transform, clip, region } => clipped(&mut ctx, region.as_ref(), |ctx| { + PlacedItem::Text { index, transform, clip, region } => clipped(&mut ctx, region.as_deref(), |ctx| { let Some(Item::Text(t)) = pic.items.get(index) else { return }; if let Some(c) = clip { ctx.push_clip_layer(&c.to_path(0.1)); @@ -281,7 +281,14 @@ fn draw_text(ctx: &mut RenderContext, t: &Text, m: Affine) { let s = em / g.face.upem.max(1.0); let skew = if italic && !g.face.italic { Affine::new([1.0, 0.0, -0.2, 1.0, 0.0, 0.0]) } else { Affine::IDENTITY }; let gm = at * Affine::translate((g.x, g.y)) * Affine::scale_non_uniform(x_scale, 1.0) * skew * Affine::scale(s); + let sc = scale_of(gm); + if !sc.is_finite() || sc > 1e5 { + continue; + } let p: BezPath = gm * path.as_ref().clone(); + if p.elements().len() > 10_000 || p.bounding_box().width() > 1e6 || p.bounding_box().height() > 1e6 { + continue; + } ctx.fill_path(&p); if bold && g.face.weight < 600.0 { ctx.set_stroke(kurbo::Stroke::new(em * 0.025 * scale_of(at)).with_join(kurbo::Join::Round));