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
2026-10-24 10:37:03 -06:00 · 2026-10-24 05:06:04 -04:00
parent c955407373
commit c55a9f771e
6 changed files with 168 additions and 32 deletions
@@ -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::<Op>(),
            // 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 {
@@ -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<P> {
+    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.
@@ -222,6 +222,7 @@ pub(crate) struct Plus {
    /// Convex device clip polygon; None is unclipped.
    clip: Area,
    objects: Vec<Option<Obj>>,
+    object_bytes: usize,
    stack: Vec<Saved>,
    partial: Option<Partial>,
    /// 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<Obj>) {
-        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::<Op>()),
+            _ => 0,
        }
    }

@@ -1282,11 +1312,18 @@ fn decode_compressed(bytes: &[u8]) -> Option<Rgba> {
    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<Vec<u16>> {
    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::<f64>() <= 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::<f64>();
+
    let mut out: Vec<Vec<u16>> = Vec::new();
    let mut cur: Vec<u16> = 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);
@@ -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<BezPath> },
+    Path { path: BezPath, fill: Option<[u8; 4]>, stroke: Option<([u8; 4], f64)>, even_odd: bool, region: Option<Arc<BezPath>> },
    /// `Picture::items[index]`, a bitmap, stretched into `rect`.
-    Bitmap { index: usize, rect: Rect, region: Option<BezPath> },
+    Bitmap { index: usize, rect: Rect, region: Option<Arc<BezPath>> },
    /// `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<BezPath> },
+    Pattern { index: usize, path: BezPath, even_odd: bool, tile: Affine, region: Option<Arc<BezPath>> },
    /// `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<Rect>, region: Option<BezPath> },
+    Text { index: usize, transform: Affine, clip: Option<Rect>, region: Option<Arc<BezPath>> },
 }

 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<Option<Arc<BezPath>>>)> = None;
+        let mut get_region = |reg: &Option<Vec<[f32; 4]>>| -> Option<Option<Arc<BezPath>>> {
+            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<Vec<[f32; 4]>>) -> Option<Option<BezPath>> {
+fn placed_region(tf: &Affine, region: &Option<Vec<[f32; 4]>>) -> Option<Option<Arc<BezPath>>> {
    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<Vec<[f32; 4]>>) -> Option<Option<B
            path.extend(r.to_path(0.1));
        }
    }
-    (!path.is_empty()).then_some(Some(path))
+    (!path.is_empty()).then_some(Some(Arc::new(path)))
 }

 /// Segments as a kurbo path. A segment after a close opens a new subpath at the last start point.
@@ -2022,3 +2022,27 @@ fn emfplus_clear_paints_the_whole_frame_and_transparent_clears_nothing() {
    // No frame: nothing to clear.
    assert_eq!(plus_pic(vec![clear(0xFFFF_FFFF), dot]).items.len(), 1);
 }
+
+#[test]
+fn emfplus_wrap_line_linear_performance() {
+    let font = prec(0x4008, 0x0600, &font_obj("Arial", 10.0, 0, 0));
+    // Repeated words that wrap across lines
+    let text = "word ".repeat(500);
+    let s = prec(
+        0x401C,
+        0x8000,
+        &cat(vec![h32(0xFF00_0000), h32(9), h32(text.encode_utf16().count() as u32), f32s(&[0.0, 0.0, 100.0, 1000.0]), u16s(&text)]),
+    );
+    let fill = prec(0x400A, 0x8000, &cat(vec![h32(0xFFFF_FFFF), h32(1), f32s(&[0.0, 0.0, 100.0, 1000.0])]));
+    let p = plus_pic(vec![fill, font, s]);
+    let text_items = p.items.iter().filter(|i| matches!(i, Item::Text(_))).count();
+    assert!(text_items > 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");
+}
@@ -51,7 +51,7 @@ pub fn rasterize(bytes: &[u8], side: u32) -> Option<image::RgbaImage> {
    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<image::RgbaImage> {
                    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<image::RgbaImage> {
                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<image::RgbaImage> {
                    }
                }
            }),
-            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));