From d1288e30194b55b7e09e478178b5abef863018a1 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Fri, 23 Oct 2026 21:53:51 -0400 Subject: [PATCH] Security: text layout, shaping, font name and embedding bounds (#3880) * Audit: text layout, shaping, fonts and embedding bounds * Audit: text bitmap chunk walk without overflow * Test: PNG metadata chunk walk * Security: keep the word minimum in wrap regions; allow uncompressed iTXt text A wrap shape can narrow the first baseline of a rectangular region: place_at_least then relaxed the minimum to 0 (the region still counted as "all lines equally wide") and picked the narrow span, breaking a word on that baseline even though a lower one had room for it whole. Treat the width as constant only when the region has no wrap shapes. Also, PNG iTXt chunks may be stored uncompressed: only genuinely compressed text (flag 1, conservatively anything that does not parse) counts as compressed metadata now. --- crates/text/src/bitmap.rs | 76 +++++++++++++++++++++++++++++++++++++ crates/text/src/colr.rs | 18 +++++++++ crates/text/src/embed.rs | 10 ++++- crates/text/src/features.rs | 9 ++++- crates/text/src/fontdb.rs | 12 +++++- crates/text/src/layout.rs | 49 +++++++++++++++++------- crates/text/src/shape.rs | 6 +-- 7 files changed, 160 insertions(+), 20 deletions(-) diff --git a/crates/text/src/bitmap.rs b/crates/text/src/bitmap.rs index 31b280e89..1c34bfe51 100644 --- a/crates/text/src/bitmap.rs +++ b/crates/text/src/bitmap.rs @@ -53,6 +53,9 @@ pub(crate) fn glyph_image(font: &skrifa::FontRef<'_>, upem: f64, gid: u32) -> Op let BitmapData::Png(png) = g.data else { return None }; // The size the PNG decodes to is its header's (`CBDT`'s own metrics may say otherwise). let (w, h) = png_size(png)?; + if has_compressed_metadata(png) { + return None; + } let (ppem_x, ppem_y) = (f64::from(g.ppem_x), f64::from(g.ppem_y)); if png.len() > MAX_PNG_BYTES || w > MAX_SIDE || h > MAX_SIDE || !(ppem_x >= 1.0 && ppem_y >= 1.0 && ppem_x.is_finite() && ppem_y.is_finite()) { return None; @@ -76,6 +79,34 @@ pub(crate) fn glyph_image(font: &skrifa::FontRef<'_>, upem: f64, gid: u32) -> Op Some(GlyphImage { blob: ImageBlob::png(png.to_vec()), width: w, height: h, xf: Affine::new([sx, 0.0, 0.0, sy, left, -top]) }) } +/// Whether PNG `bytes` has a chunk of compressed metadata (a colour profile, compressed text): a +/// decoder unpacks it before it reads the image, whatever size the image says. +fn has_compressed_metadata(bytes: &[u8]) -> bool { + let mut at: usize = 8; + while let Some(head) = at.checked_add(8).and_then(|end| bytes.get(at..end)) { + if matches!(&head[4..], b"iCCP" | b"zTXt") { + return true; + } + let len32 = u32::from_be_bytes([head[0], head[1], head[2], head[3]]); + if &head[4..] == b"iTXt" && !itxt_uncompressed(bytes, at, len32) { + return true; + } + // The chunk's data and its checksum follow its length and type. + at = at.saturating_add(len32 as usize).saturating_add(12); + } + false +} + +/// Whether an `iTXt` chunk's `data` is stored uncompressed: the compression flag (0 = plain text) +/// follows the keyword's nul. Anything that does not parse as that layout may still be compressed, so +/// the caller treats it as such. The scan stays inside the chunk's declared data: beyond it lie other +/// chunks, and a byte there could pose as the keyword's terminator. +fn itxt_uncompressed(bytes: &[u8], head_at: usize, len: u32) -> bool { + let from = head_at + 8; + let data = bytes.get(from..from.saturating_add(len as usize)).unwrap_or(&[]); + data.iter().position(|&b| b == 0).and_then(|kw| data.get(kw + 1)).map_or(false, |&flag| flag == 0) +} + /// The width and height in PNG `bytes`' header (its signature, then the IHDR chunk); `None` when /// it doesn't read or a side is 0. fn png_size(bytes: &[u8]) -> Option<(u32, u32)> { @@ -86,3 +117,48 @@ fn png_size(bytes: &[u8]) -> Option<(u32, u32)> { let (w, h) = (be(16)?, be(20)?); (w > 0 && h > 0).then_some((w, h)) } + +#[cfg(test)] +mod tests { + use super::*; + + /// A PNG signature and the chunks `types` (each with 2 bytes of data and a checksum). + fn png(types: &[&[u8; 4]]) -> Vec { + let mut b = b"\x89PNG\r\n\x1a\n".to_vec(); + for t in types { + b.extend([0, 0, 0, 2]); + b.extend(*t); + b.extend([0; 6]); + } + b + } + + #[test] + fn compressed_metadata_chunks_are_found_wherever_they_are() { + assert!(!has_compressed_metadata(&png(&[b"IHDR", b"IDAT", b"IEND"]))); + assert!(has_compressed_metadata(&png(&[b"IHDR", b"iCCP", b"IDAT", b"IEND"]))); + assert!(has_compressed_metadata(&png(&[b"IHDR", b"IDAT", b"zTXt"]))); + // A length past the end ends the walk. + let mut cut = png(&[b"IHDR"]); + cut.extend([255, 255, 255, 255, b'I', b'D', b'A', b'T']); + assert!(!has_compressed_metadata(&cut)); + } + + /// A PNG signature and one chunk `t` carrying `data` (the stand-in CRC is never read: the + /// scan walks lengths and types only). + fn png_chunk(t: &[u8; 4], data: &[u8]) -> Vec { + let mut b = b"\x89PNG\r\n\x1a\n".to_vec(); + b.extend(u32::try_from(data.len()).unwrap_or(u32::MAX).to_be_bytes()); + b.extend(t); + b.extend(data); + b.extend([0; 4]); + b + } + + #[test] + fn an_uncompressed_itxt_needs_no_decompression() { + assert!(!has_compressed_metadata(&png_chunk(b"iTXt", b"hi\0\0\0\0")), "compression flag 0 stores plain text"); + assert!(has_compressed_metadata(&png_chunk(b"iTXt", b"hi\0\x01\0\0\0")), "compression flag 1 is unpacked by the decoder"); + assert!(has_compressed_metadata(&png_chunk(b"iTXt", b"hi")), "no keyword nul: conservative stays safe"); + } +} diff --git a/crates/text/src/colr.rs b/crates/text/src/colr.rs index e8ef12712..9045a4f55 100644 --- a/crates/text/src/colr.rs +++ b/crates/text/src/colr.rs @@ -22,6 +22,10 @@ use crate::fontdb::FlipPen; /// The most nodes a colour glyph's tree may have: past it, a font's glyph draws its outline. const MAX_COLR_NODES: usize = 4096; + +/// The most path elements the clips and fills of a colour glyph's tree may hold in all: layers +/// that repeat one large outline each hold a copy of it. +const MAX_COLR_POINTS: usize = 1 << 20; /// The deepest the groups of a colour glyph may nest. const MAX_COLR_DEPTH: usize = 64; /// The pixels per em an image of a colour glyph is drawn at. @@ -134,6 +138,8 @@ struct Tree<'a> { transforms: Vec, groups: Vec, nodes: usize, + /// The path elements of the clips and fills so far. + points: usize, /// What VectorCraft has no form for, the first one met: the glyph becomes an image. unsupported: Option<&'static str>, /// The graph is past [`MAX_COLR_NODES`] or [`MAX_COLR_DEPTH`]. @@ -155,6 +161,7 @@ impl<'a> Tree<'a> { transforms: vec![FLIP], groups: vec![root], nodes: 0, + points: 0, unsupported: None, too_large: false, painted: None, @@ -174,6 +181,13 @@ impl<'a> Tree<'a> { } } + /// Count `path` against [`MAX_COLR_POINTS`]: whether it may be kept. + fn charge(&mut self, path: &BezPath) -> bool { + self.points = self.points.saturating_add(path.elements().len()); + self.too_large |= self.points > MAX_COLR_POINTS; + !self.too_large + } + fn open(&mut self, group: Open) { self.too_large |= self.groups.len() >= MAX_COLR_DEPTH; self.groups.push(group); @@ -309,6 +323,9 @@ impl<'a> Tree<'a> { /// `path` (in the tree's space) filled with `brush` (in the current transform's). fn fill_path(&mut self, path: &BezPath, brush: &Brush<'_>) { + if !self.charge(path) { + return; + } let b = path.bounding_box(); self.painted = Some(self.painted.map_or(b, |p| p.union(b))); let Some((paint, opacity)) = self.paint(brush) else { return }; @@ -344,6 +361,7 @@ impl ColorPainter for Tree<'_> { fn push_clip_glyph(&mut self, glyph_id: GlyphId) { let mut clip = outline(self.font, self.location, glyph_id); clip.apply_affine(self.transform()); + self.charge(&clip); self.open(Open { clip: Some(clip), blend: None, children: vec![], flooded: false }); } diff --git a/crates/text/src/embed.rs b/crates/text/src/embed.rs index 6615d10c9..6349391d9 100644 --- a/crates/text/src/embed.rs +++ b/crates/text/src/embed.rs @@ -223,6 +223,9 @@ fn checksum(data: &[u8]) -> u32 { words.iter().chain([&last]).fold(0u32, |s, w| s.wrapping_add(u32::from_be_bytes(*w))) } +/// The most bytes of a font file written. +const MAX_FONT_BYTES: usize = 256 << 20; + /// An OpenType file of `tables` (unique tags) with checksums and the `head` checksum adjustment. fn sfnt(version: u32, mut tables: Vec<(Tag, Cow<'_, [u8]>)>) -> Option> { tables.sort_by_key(|t| t.0); @@ -230,7 +233,12 @@ fn sfnt(version: u32, mut tables: Vec<(Tag, Cow<'_, [u8]>)>) -> Option> let n = u16::try_from(tables.len()).ok().filter(|n| *n > 0)?; let sel = n.ilog2() as u16; let range = 2u16.checked_pow(u32::from(sel))?.checked_mul(16)?; - let mut out = Vec::with_capacity(12 + 16 * tables.len() + tables.iter().map(|t| t.1.len() + 3).sum::()); + // Tables may alias one range of the input: the file is no larger than a font file may be. + let size = tables.iter().try_fold(12 + 16 * tables.len(), |sum, t| sum.checked_add(t.1.len().checked_add(3)?))?; + if size > MAX_FONT_BYTES { + return None; + } + let mut out = Vec::with_capacity(size); out.extend(version.to_be_bytes()); for v in [n, range, sel, n.checked_mul(16)? - range] { out.extend(v.to_be_bytes()); diff --git a/crates/text/src/features.rs b/crates/text/src/features.rs index 075268465..dd71a20c9 100644 --- a/crates/text/src/features.rs +++ b/crates/text/src/features.rs @@ -46,6 +46,9 @@ pub struct OtFeatures { pub extra: u32, } +/// The most entries of a style's feature list read: every segment of its text reads the list again. +const MAX_STYLE_FEATURES: usize = 256; + /// Features beyond the named fields, off unless asked for, each bit `i` of [`OtFeatures::extra`]: /// stylistic sets `ss01`–`ss20`, stylistic alternates `salt`, titling `titl`, proportional `pnum` /// and lining `lnum` figures, superscript `sups` and subscript `subs`. @@ -146,13 +149,15 @@ impl OtFeatures { /// The harfrust features for text in style `st`. pub(crate) fn resolve(&self, st: &CharStyle) -> Vec { // The character's own OpenType settings override the layout-wide ones. - let own = self.with_tags(st.features.iter().map(String::as_str)); + // (A style lists a few features; one with thousands, in a document, is read for the first.) + let listed = st.features.get(..MAX_STYLE_FEATURES).unwrap_or(&st.features); + let own = self.with_tags(listed.iter().map(String::as_str)); let s = &own; let mut v = Vec::with_capacity(8); if st.kerning.is_some() { v.push(f(b"kern", false)); } - let liga = s.ligatures && (!ligatures_suppressed_by(st.tracking) || explicit_ligatures(&st.features)); + let liga = s.ligatures && (!ligatures_suppressed_by(st.tracking) || explicit_ligatures(listed)); if !liga { v.push(f(b"liga", false)); v.push(f(b"clig", false)); diff --git a/crates/text/src/fontdb.rs b/crates/text/src/fontdb.rs index ff292f96c..9642bf7dd 100644 --- a/crates/text/src/fontdb.rs +++ b/crates/text/src/fontdb.rs @@ -639,6 +639,9 @@ fn name(font: &skrifa::FontRef<'_>, ids: &[StringId]) -> Option { ids.iter().find_map(|id| font.localized_strings(*id).english_or_first().map(|s| s.to_string()).filter(|s| !s.is_empty())) } +/// The most distinct legacy family (and style) names of a font used as keys. +const MAX_LEGACY_NAMES: usize = 64; + /// Every localized string of `id`, with its language. fn localized(font: &skrifa::FontRef<'_>, id: StringId) -> Vec<(Option, String)> { font.localized_strings(id).map(|s| (s.language().map(str::to_owned), s.to_string())).filter(|(_, s)| !s.is_empty()).collect() @@ -691,7 +694,14 @@ struct FaceKeys { impl FaceKeys { fn of(font: &skrifa::FontRef<'_>, family: &str, style: &str) -> Self { let (typo_family, typo_style) = (all_names(font, StringId::TYPOGRAPHIC_FAMILY_NAME), all_names(font, StringId::TYPOGRAPHIC_SUBFAMILY_NAME)); - let (legacy_family, legacy_style) = (localized(font, StringId::FAMILY_NAME), localized(font, StringId::SUBFAMILY_NAME)); + let (mut legacy_family, mut legacy_style) = (localized(font, StringId::FAMILY_NAME), localized(font, StringId::SUBFAMILY_NAME)); + // Records repeat (platforms, encodings) and are paired below: once each, and no more than a + // font has any use for, or the pairs multiply. + for names in [&mut legacy_family, &mut legacy_style] { + names.sort(); + names.dedup(); + names.truncate(MAX_LEGACY_NAMES); + } // Without typographic names, the legacy ones are the family's. let family_names: Vec<&str> = if typo_family.is_empty() { legacy_family.iter().map(|(_, f)| f.as_str()).collect() diff --git a/crates/text/src/layout.rs b/crates/text/src/layout.rs index 6dfa485b2..7390897dc 100644 --- a/crates/text/src/layout.rs +++ b/crates/text/src/layout.rs @@ -46,7 +46,11 @@ struct Ctx<'a> { impl Ctx<'_> { fn style_at(&self, b: usize) -> &CharStyle { - let find = |b: usize| self.runs.iter().find(|(r, _)| r.start <= b && b < r.end).map(|(_, s)| *s); + // The runs follow one another: the first that ends after `b` is the only one that can hold it. + let find = |b: usize| { + let i = self.runs.partition_point(|(r, _)| r.end <= b); + self.runs.get(i).filter(|(r, _)| r.start <= b).map(|(_, s)| *s) + }; find(b).or_else(|| b.checked_sub(1).and_then(find)).or_else(|| self.runs.first().map(|(_, s)| *s)).unwrap_or(&self.default) } @@ -1304,8 +1308,10 @@ impl Pen<'_> { if baseline + est.desc > r.bottom() + 0.01 { break; } - // A rectangle's lines are all as wide: none below has more room for the word. - let min = if r.rect || r.polys.is_empty() { 0.0 } else { min }; + // A rectangle's lines are all as wide: none below has more room for the word + // (until wrap shapes can narrow the first baseline: they stay bounded). + let constant = (r.rect || r.polys.is_empty()) && r.wraps.is_empty(); + let min = if constant { 0.0 } else { min }; let fits = |&(a, b): &(f64, f64)| { let room = b - a - ind_l - ind_r; room > est.asc.max(1.0) && room + EPS >= min @@ -1313,6 +1319,8 @@ impl Pen<'_> { if r.wraps.is_empty() { match r.span(baseline - est.asc, baseline + est.desc) { Some(s) if fits(&s) => return Some((baseline, s.0, s.1, false)), + // Too narrow here is too narrow all the way down the cell: next cell. + Some(_) if constant => break, _ => baseline += step, } } else { @@ -1430,10 +1438,13 @@ fn leader_outline(db: &FontDb, g: &SGlyph, leader: &str, origin: f64, from: f64, /// left out, the hyphen of a soft hyphen in), the least room a line needs not to break inside it. /// A list marker at `i` (glyphs of length 0) counts whole, with the word after it. fn word_width(g: &[SGlyph], i: usize) -> f64 { + /// Glyphs measured at most: a word wrapped over many lines is measured again from each line's + /// start, and one this long is broken inside whatever cell it meets. + const MAX_GLYPHS: usize = 1024; let rest = g.get(i..).unwrap_or_default(); let marker = rest.iter().take_while(|gl| gl.len == 0).count(); let mut w: f64 = rest.iter().take(marker).map(|gl| gl.adv).sum(); - for gl in rest.iter().skip(marker) { + for gl in rest.iter().skip(marker).take(MAX_GLYPHS) { if gl.is_space() { break; } @@ -1879,14 +1890,15 @@ fn hyphen_breaks(text: &str, g: &[SGlyph], ws: usize, we: usize, opts: &Hyphenat let (s, e) = (g[a].byte, g[b - 1].byte + g[b - 1].len); let Some(word) = text.get(s..e) else { return vec![] }; let offs: Vec = word.char_indices().map(|(o, _)| s + o).collect(); - hyphen_points_with(word, opts) - .into_iter() - .filter_map(|ci| { - let byte = *offs.get(ci)?; - // Only at cluster starts (not inside a ligature). - (a + 1..b).find(|&k| g[k].byte == byte && g[k - 1].byte != byte) - }) - .collect() + // The first glyph of each cluster start (not inside a ligature), by its byte: one pass, not a + // search for each point. + let mut starts = std::collections::HashMap::new(); + for k in a + 1..b { + if g[k].byte != g[k - 1].byte { + starts.entry(g[k].byte).or_insert(k); + } + } + hyphen_points_with(word, opts).into_iter().filter_map(|ci| starts.get(offs.get(ci)?).copied()).collect() } /// Break candidates for the every-line composer: hyphenation points under `hyphenate` (the @@ -2904,6 +2916,11 @@ fn ruby_layout(cx: &mut Ctx<'_>, t: &TextObject) { overhang: bool, } let mut pieces: Vec = Vec::new(); + // The glyphs by their byte, to find a span's without a scan of them all for each span. + let byte_of = |i: usize| cx.out.glyphs.get(i).map_or(0, |g| g.byte); + let mut by_byte: Vec = (0..cx.out.glyphs.len()).collect(); + by_byte.sort_by_key(|&i| byte_of(i)); + let longest = cx.out.glyphs.iter().map(|g| g.len.max(1)).max().unwrap_or(1); for span in &t.ruby { let (start, end) = (span.start.min(text_len), span.end.min(text_len)); if start >= end || span.text.trim().is_empty() { @@ -2912,7 +2929,13 @@ fn ruby_layout(cx: &mut Ctx<'_>, t: &TextObject) { // The base glyphs of the span, by line; a glyph's cluster is its byte range (`len`, or // one byte for generated glyphs). A list marker is not part of the base. let mut by_line: BTreeMap> = BTreeMap::new(); - for (i, g) in cx.out.glyphs.iter().enumerate() { + let hi = by_byte.partition_point(|&i| byte_of(i) < end); + let lo = by_byte.partition_point(|&i| byte_of(i).saturating_add(longest) <= start).min(hi); + let mut hits: Vec = by_byte.get(lo..hi).unwrap_or_default().to_vec(); + // In glyph order, as the scan found them. + hits.sort_unstable(); + for i in hits { + let Some(g) = cx.out.glyphs.get(i) else { continue }; let b = g.byte.saturating_add(g.len.max(1)); if g.byte < end && b > start && cx.out.marker_of(i).is_none() { by_line.entry(g.line).or_default().push(i); diff --git a/crates/text/src/shape.rs b/crates/text/src/shape.rs index 1fa96c87a..45ad8ee24 100644 --- a/crates/text/src/shape.rs +++ b/crates/text/src/shape.rs @@ -371,7 +371,7 @@ pub(crate) fn shape_range( let small_caps = st.small_caps.is_some() && !st.all_caps; // Split into segments by font coverage (and case, for Small Caps). let mut seg = Segment { range: a..a, runs, source_runs, st, face: primary.clone(), small: false, level: level_at(a) }; - let mut cache: Vec<(char, Arc)> = Vec::new(); + let mut cache: std::collections::HashMap> = std::collections::HashMap::new(); let mut script = Script::Common; for (i, c) in text[a..b].char_indices() { let i = a + i; @@ -384,11 +384,11 @@ pub(crate) fn shape_range( let covered = c.is_control() || pmap.as_ref().is_none_or(|m| m.map(c).is_some()); let face = if covered { primary.clone() - } else if let Some((_, f)) = cache.iter().find(|(k, _)| *k == c) { + } else if let Some(f) = cache.get(&c) { f.clone() } else { let f = db.fallback_for_style(c, primary.id(), primary.weight, primary.italic).unwrap_or_else(|| primary.clone()); - cache.push((c, f.clone())); + cache.insert(c, f.clone()); f }; let small = small_caps && c.is_lowercase();