From dc4bbe86f64a3126173b2f7af930132f287e9571 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Thu, 22 Oct 2026 17:57:39 -0400 Subject: [PATCH] security: bound font outline geometry and MTEXT line scanning (CWE-400/407) (#1852) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * security: bound font outline geometry and MTEXT line scanning (CWE-400/407) Two font/text paths let crafted input monopolise the rendering thread: - Glyph shaping copied a glyph's full contour set for every placed character with no cumulative budget, so a compact CFF/OTF font with a large expanded outline, repeated across a short string, could grow the shaped result without bound. `shape` now charges retained outline points against a cumulative per-line budget and stops placing geometry once it is exhausted (advances still accumulate, so the width stays sensible) (row 9). - Default MTEXT layout recomputed `line.text.chars().count()` before appending each segment or space, rescanning the whole accumulated line every time — multiplicative on a long line. The line now keeps a running character count, updated only from newly appended text (row 20). New tests cover the MTEXT layout; all font tests pass and render still builds, clippy clean. Addresses findings rows 9, 20. * fix: budget glyph geometry before outline placement * fix: skip empty outlines without text metadata --------- Co-authored-by: skylartaylor <26588250+skylartaylor@users.noreply.github.com> --- crates/fonts/src/complex.rs | 10 +-- crates/fonts/src/lib.rs | 4 + crates/fonts/src/mtext.rs | 21 +++++- crates/fonts/src/ttf.rs | 143 +++++++++++++++++++++++++++++++----- 4 files changed, 151 insertions(+), 27 deletions(-) diff --git a/crates/fonts/src/complex.rs b/crates/fonts/src/complex.rs index b479590e..fdfef151 100644 --- a/crates/fonts/src/complex.rs +++ b/crates/fonts/src/complex.rs @@ -68,6 +68,7 @@ fn shape_chars(primary: Option<&Face>, chars: &[(char, bool, bool)], height: f64 let cs: Vec = chars.iter().map(|c| c.0).collect(); let flags = |i: usize| chars.get(i).map(|c| (c.1, c.2)).unwrap_or((false, false)); let mut out = Shaped::default(); + let mut points_left = ttf::MAX_SHAPED_POINTS; let mut spans: Vec<(f64, f64, bool, bool)> = Vec::new(); let mut x = 0.0; // Whether the rightmost glyph so far is a stroke-font glyph. @@ -102,11 +103,10 @@ fn shape_chars(primary: Option<&Face>, chars: &[(char, bool, bool)], height: f64 let outline = ttf::glyph_by_id(face, g.gid); let k = h / outline.cap * wf; let dy = g.dy * h / outline.cap; - let mut contours = ttf::place_glyph(&outline, x + g.dx * k, h, wf, shear); - for p in contours.iter_mut().flatten() { - *p += Vec2::new(dy * shear, dy); - } - if !contours.is_empty() { + if let Some(mut contours) = ttf::place_glyph_budgeted(&outline, x + g.dx * k, h, wf, shear, &mut points_left) { + for p in contours.iter_mut().flatten() { + *p += Vec2::new(dy * shear, dy); + } out.glyphs.push(contours); } let adv = g.advance * k; diff --git a/crates/fonts/src/lib.rs b/crates/fonts/src/lib.rs index 3f86f817..13af0564 100644 --- a/crates/fonts/src/lib.rs +++ b/crates/fonts/src/lib.rs @@ -334,7 +334,11 @@ pub struct TextGlyph { impl Shaped { /// Add a glyph outline and, when it is a font glyph, its [`TextGlyph`]. + /// An empty outline is retained only when text metadata needs its slot. pub(crate) fn push_glyph(&mut self, contours: Vec>, text: Option) { + if contours.is_empty() && text.is_none() { + return; + } if let Some(t) = text { self.text.push(TextGlyph { outline: self.glyphs.len(), ..t }); } diff --git a/crates/fonts/src/mtext.rs b/crates/fonts/src/mtext.rs index 1d53dc91..916bdf33 100644 --- a/crates/fonts/src/mtext.rs +++ b/crates/fonts/src/mtext.rs @@ -501,6 +501,9 @@ struct Line { width: f64, max_h: f64, text: String, + /// Running character count of `text`, kept incrementally so appending a segment need not + /// rescan the whole accumulated line (which is multiplicative on a long line) (CWE-407). + chars: usize, /// Index of the paragraph the line belongs to. para: usize, } @@ -580,9 +583,10 @@ pub fn layout_mtext_with(contents: &str, p: &MTextParams) -> MTextLayout { finish(&mut lines, &mut line, fmt(last_fmt), para); } else if !line.items.is_empty() || pending_space.is_some() { if let Some(f) = pending_space { - let at = line.text.chars().count(); + let at = line.chars; line.items.push(Item { seg: None, fmt: f, x: line.width, w: sw, at }); line.text.push(' '); + line.chars += 1; } line.width += sw; } @@ -590,8 +594,10 @@ pub fn layout_mtext_with(contents: &str, p: &MTextParams) -> MTextLayout { for (sg, f) in segs { let fw = seg_width(sg, fmt(*f)); line.max_h = line.max_h.max(fmt(*f).height); - let at = line.text.chars().count(); - line.text.push_str(&seg_plain(sg)); + let at = line.chars; + let plain = seg_plain(sg); + line.chars += plain.chars().count(); + line.text.push_str(&plain); line.items.push(Item { seg: Some(sg.clone()), fmt: *f, x: line.width, w: fw, at }); line.width += fw; last_fmt = *f; @@ -679,6 +685,15 @@ pub fn layout_mtext_with(contents: &str, p: &MTextParams) -> MTextLayout { mod tests { use super::*; + #[test] + fn line_char_offsets_track_incrementally() { + // A multi-word line lays out correctly even though the per-line character count is now + // maintained incrementally rather than rescanned per segment (CWE-407, row 20). + let l = layout_mtext("alpha beta gamma", Vec2::ZERO, 1.0, 0.0, 1, 0.0, 1.0); + assert_eq!(l.lines.len(), 1, "no wrap width: one line"); + assert_eq!(l.lines[0], "alpha beta gamma"); + } + #[test] fn strips_formatting() { assert_eq!(plain_mtext("{\\fArial|b1;Bold}\\Pline 2"), "Bold\nline 2"); diff --git a/crates/fonts/src/ttf.rs b/crates/fonts/src/ttf.rs index 62759c1b..aedcbb07 100644 --- a/crates/fonts/src/ttf.rs +++ b/crates/fonts/src/ttf.rs @@ -107,6 +107,9 @@ const MAX_FIRST_WORD_FILES: usize = 24; const MAX_LOADED_BYTES: usize = 256 << 20; const MAX_CACHED_NAMES: usize = 4096; const MAX_CACHED_CHARS: usize = 50_000; +/// Cumulative outline points one shaped line may retain, so an oversized expanded glyph repeated +/// across a string cannot grow the result without bound (CWE-400). +pub(crate) const MAX_SHAPED_POINTS: usize = 8_000_000; /// Fonts known to cover many scripts (CJK first), tried first for missing glyphs. Matched as /// substrings of the normalised file stem. @@ -595,6 +598,7 @@ pub fn fallback_for(c: char) -> Option { #[derive(Default)] pub(crate) struct GlyphOutline { pub(crate) contours: Arc<[Vec]>, + points: usize, pub(crate) advance: f64, pub(crate) cap: f64, /// The font maps the character (false: the missing-glyph shape). @@ -614,22 +618,46 @@ enum GlyphKey { /// Cached glyph outlines keyed by (font bytes address, length, face, glyph). Fonts live for the /// whole process in the font table, so the address identifies them. A read-write lock: display /// lists are built on all cores and nearly every lookup is a hit, which a mutex serialized. -type GlyphCache = HashMap<(usize, usize, u32, GlyphKey), Arc>; +#[derive(Default)] +struct GlyphCache { + glyphs: HashMap<(usize, usize, u32, GlyphKey), Arc>, + points: usize, +} fn glyph_cache() -> &'static RwLock { static C: OnceLock> = OnceLock::new(); - C.get_or_init(|| RwLock::new(HashMap::new())) + C.get_or_init(|| RwLock::new(GlyphCache::default())) } const MAX_CACHED_GLYPHS: usize = 40_000; const UPEM: f32 = 1000.0; +#[derive(Default)] struct Pen { contours: Vec>, cur: Vec, + points: usize, + truncated: bool, } +const MAX_OUTLINE_POINTS: usize = 65_536; + impl Pen { + fn push(&mut self, point: Vec2) { + if self.truncated { + return; + } + if self.points == MAX_OUTLINE_POINTS { + // An incomplete contour is not safe to fill; retain only the advance for this glyph. + self.truncated = true; + self.cur.clear(); + self.contours.clear(); + return; + } + self.points += 1; + self.cur.push(point); + } + fn pt(x: f32, y: f32) -> Vec2 { Vec2::new(f64::from(x), f64::from(y)) } @@ -638,9 +666,11 @@ impl Pen { if let (Some(f), Some(l)) = (self.cur.first().copied(), self.cur.last().copied()) && f != l { - self.cur.push(f); + self.push(f); + } + if !self.truncated { + self.contours.push(std::mem::take(&mut self.cur)); } - self.contours.push(std::mem::take(&mut self.cur)); } else { self.cur.clear(); } @@ -650,10 +680,10 @@ impl Pen { impl OutlinePen for Pen { fn move_to(&mut self, x: f32, y: f32) { self.flush(); - self.cur.push(Self::pt(x, y)); + self.push(Self::pt(x, y)); } fn line_to(&mut self, x: f32, y: f32) { - self.cur.push(Self::pt(x, y)); + self.push(Self::pt(x, y)); } fn quad_to(&mut self, cx0: f32, cy0: f32, x: f32, y: f32) { let a = self.cur.last().copied().unwrap_or_default(); @@ -662,7 +692,7 @@ impl OutlinePen for Pen { for i in 1..=6 { let t = f64::from(i) / 6.0; let u = 1.0 - t; - self.cur.push(a * (u * u) + c * (2.0 * u * t) + b * (t * t)); + self.push(a * (u * u) + c * (2.0 * u * t) + b * (t * t)); } } fn curve_to(&mut self, cx0: f32, cy0: f32, cx1: f32, cy1: f32, x: f32, y: f32) { @@ -673,7 +703,7 @@ impl OutlinePen for Pen { for i in 1..=8 { let t = f64::from(i) / 8.0; let u = 1.0 - t; - self.cur.push(a * (u * u * u) + c0 * (3.0 * u * u * t) + c1 * (3.0 * u * t * t) + b * (t * t * t)); + self.push(a * (u * u * u) + c0 * (3.0 * u * u * t) + c1 * (3.0 * u * t * t) + b * (t * t * t)); } } fn close(&mut self) { @@ -714,7 +744,7 @@ pub fn glyph_contours(face: &Face, gid: u16) -> Arc<[Vec]> { fn outline(face: &Face, k: GlyphKey) -> Arc { let key = (face.bytes.as_ptr() as usize, face.bytes.len(), face.index, k); - if let Some(g) = glyph_cache().read().unwrap_or_else(PoisonError::into_inner).get(&key) { + if let Some(g) = glyph_cache().read().unwrap_or_else(PoisonError::into_inner).glyphs.get(&key) { return g.clone(); } let Some(f) = face.font() else { return Arc::new(GlyphOutline { cap: 1.0, ..Default::default() }) }; @@ -723,19 +753,24 @@ fn outline(face: &Face, k: GlyphKey) -> Arc { GlyphKey::Id(id) => Some(GlyphId::new(id)), }; let gid = mapped.unwrap_or_default(); - let mut pen = Pen { contours: Vec::new(), cur: Vec::new() }; + let mut pen = Pen::default(); if let Some(g) = f.outline_glyphs().get(gid) { let _ = g.draw(DrawSettings::unhinted(Size::new(UPEM), LocationRef::default()), &mut pen); pen.flush(); } let advance = f64::from(f.glyph_metrics(Size::new(UPEM), LocationRef::default()).advance_width(gid).unwrap_or(UPEM * 0.5)); let (cap, mapped, gid) = (cap_height(&f), mapped.is_some(), gid.to_u32()); - let g = Arc::new(GlyphOutline { contours: pen.contours.into(), advance, cap, mapped, gid, face: Some(face.clone()) }); + let points = pen.contours.iter().map(Vec::len).sum(); + let g = Arc::new(GlyphOutline { points, contours: pen.contours.into(), advance, cap, mapped, gid, face: Some(face.clone()) }); let mut cache = glyph_cache().write().unwrap_or_else(PoisonError::into_inner); - if cache.len() > MAX_CACHED_GLYPHS { - cache.clear(); + if cache.glyphs.len() >= MAX_CACHED_GLYPHS || cache.points.saturating_add(points) > MAX_SHAPED_POINTS { + cache.glyphs.clear(); + cache.points = 0; } - cache.insert(key, g.clone()); + if let Some(old) = cache.glyphs.insert(key, g.clone()) { + cache.points -= old.points; + } + cache.points += points; g } @@ -777,6 +812,11 @@ pub(crate) fn text_glyph_at(g: &GlyphOutline, c: char, origin: Vec2, scale: f64, }) } +pub(crate) fn place_glyph_budgeted(g: &GlyphOutline, x: f64, h: f64, wf: f64, shear: f64, points_left: &mut usize) -> Option>> { + *points_left = points_left.checked_sub(g.points)?; + Some(place_glyph(g, x, h, wf, shear)) +} + /// Contours of a glyph scaled so its font's cap height is `h`, at pen position `x`. pub(crate) fn place_glyph(g: &GlyphOutline, x: f64, h: f64, wf: f64, shear: f64) -> Vec> { place_glyph_at(g, Vec2::new(x, 0.0), h / g.cap, wf, shear) @@ -814,16 +854,20 @@ pub fn shape(face: &Face, s: &str, height: f64, width_factor: f64, oblique: f64) let mut out = Shaped::default(); let mut spans = Vec::new(); let mut x = 0.0; + // Cumulative retained-point budget: a compact CFF/OTF font can expand one glyph into a huge + // outline, and a short repeated string copies it per occurrence, so cap the total geometry + // kept in the Shaped result (CWE-400). Advances still accumulate so the width stays sensible. + let mut points_left: usize = MAX_SHAPED_POINTS; for (c, under, over) in crate::decode_controls(s) { if crate::bidi::is_format_control(c) { continue; } let g = glyph_or_fallback(face, c); - if !c.is_whitespace() { - let contours = place_glyph(&g, x, h, wf, shear); - if !contours.is_empty() { - out.push_glyph(contours, text_glyph(&g, c, x, h, wf, shear)); - } + if !c.is_whitespace() + && let Some(contours) = place_glyph_budgeted(&g, x, h, wf, shear, &mut points_left) + { + // Even an empty mapped glyph needs a valid outline slot for text export. + out.push_glyph(contours, text_glyph(&g, c, x, h, wf, shear)); } let adv = g.advance * h / g.cap * wf; spans.push((x, x + adv, under, over)); @@ -878,3 +922,64 @@ mod family_tests { assert!(family_candidates(&many, "gosttypea", "gost").len() <= MAX_FAMILY_FILES + MAX_FIRST_WORD_FILES); } } + +#[cfg(test)] +mod geometry_budget_tests { + use super::*; + #[test] + fn placement_checks_source_size_and_accepts_empty_glyphs() { + let glyph = GlyphOutline { contours: vec![vec![Vec2::ZERO; 4]].into(), points: 4, cap: 1.0, ..Default::default() }; + let mut budget = 3; + assert!(place_glyph_budgeted(&glyph, 0.0, 1.0, 1.0, 0.0, &mut budget).is_none()); + assert_eq!(budget, 3); + let empty = GlyphOutline { cap: 1.0, ..Default::default() }; + assert_eq!(place_glyph_budgeted(&empty, 0.0, 1.0, 1.0, 0.0, &mut 0), Some(Vec::new())); + let mut budget = 4; + assert!(place_glyph_budgeted(&glyph, 0.0, 1.0, 1.0, 0.0, &mut budget).is_some()); + assert_eq!(budget, 0); + assert!(place_glyph_budgeted(&glyph, 0.0, 1.0, 1.0, 0.0, &mut budget).is_none()); + } + #[test] + fn flattened_outlines_are_bounded_before_caching() { + let mut pen = Pen::default(); + for _ in 0..MAX_OUTLINE_POINTS { + pen.line_to(0.0, 0.0); + } + pen.line_to(1.0, 1.0); + pen.close(); + assert!(pen.truncated); + assert!(pen.contours.is_empty() && pen.cur.is_empty()); + assert_eq!(pen.points, MAX_OUTLINE_POINTS); + } + #[test] + fn mapped_empty_glyph_keeps_text_and_a_valid_outline_slot() { + // Metadata placement does not parse the face; an empty face suffices to check the index contract. + let glyph = GlyphOutline { cap: 1.0, mapped: true, gid: 1, face: Some(Face { bytes: Arc::new(Vec::new()), index: 0 }), ..Default::default() }; + let contours = place_glyph_budgeted(&glyph, 0.0, 1.0, 1.0, 0.0, &mut 0).unwrap(); + let mut shaped = Shaped::default(); + shaped.push_glyph(contours, text_glyph(&glyph, 'x', 0.0, 1.0, 1.0, 0.0)); + assert_eq!(shaped.text.len(), 1); + assert_eq!(shaped.text[0].outline, 0); + assert!(shaped.glyphs[shaped.text[0].outline].is_empty()); + } + + #[test] + fn unmapped_empty_glyph_does_not_make_the_shape_nonempty() { + let glyph = GlyphOutline { cap: 1.0, ..Default::default() }; + let contours = place_glyph_budgeted(&glyph, 0.0, 1.0, 1.0, 0.0, &mut 0).unwrap(); + let mut shaped = Shaped::default(); + shaped.push_glyph(contours, text_glyph(&glyph, 'x', 0.0, 1.0, 1.0, 0.0)); + assert!(shaped.is_empty()); + assert!(shaped.text.is_empty()); + } + + #[test] + fn an_unmapped_glyph_with_contours_still_draws() { + let mut shaped = Shaped::default(); + let contours = vec![vec![Vec2::ZERO, Vec2::new(1.0, 1.0)]]; + shaped.push_glyph(contours.clone(), None); + assert!(!shaped.is_empty()); + assert_eq!(shaped.glyphs, vec![contours]); + assert!(shaped.text.is_empty()); + } +}