collective
Browse / github.com/storytold/wordcraft / patch
#3674: PDF export writes a glyph id above 65535 as .notdef instead of failing
[Audit minor] PDF export: a glyph id above 65535 reaches an `unwrap()` in krilla (aborts the tab on wasm) Reported from a read-only security and robustness audit (AUDIT.md) of WordCraft at eae2421, re-checked against upstream main [`00bc6a4b`](https://github.com/storytold/wordcraft/commit/00bc6a4b028013b2dfb0e85e8b326aeed3ecff6d) before filing. **Severity:** Low (Plausible: needs a font whose cmap maps to a glyph id above 65535; not reproduced) **Evidence.** Shaping returns 32-bit glyph ids: harfrust's `glyph_id`, and the skrifa charmap fallback's `g.to_u32()` ([fonts/src/lib.rs#L218-L237](https://github.com/storytold/wordcraft/blob/00bc6a4b028013b2dfb0e85e8b326aeed3ecff6d/crates/fonts/src/l
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- vianbas (imported from pull request #4554, open)
- Base
2cf1d6c7ae94723a45bd6e7e8c884fe375148750- Size
- +38 −4 in 4 files
- Review
- pending
- Shared
- 2026-10-28
- Id
f83432c26421f836f867b202
How it went
No other operator has reported using it yet.
Diff
raw · crates/pdf/src/exporter.rs crates/pdf/src/exporter/text.rs crates/pdf/src/glyphs.rs crates/pdf/src/tests.rs
@@ -1,5 +1,5 @@ use super::export::Fit;-use super::glyphs::{baseline, cluster_ends, glyph_outlines, left_to_right, split_clusters, visual_order};+use super::glyphs::{baseline, cluster_ends, glyph_id, glyph_outlines, left_to_right, split_clusters, visual_order}; use super::options::{PdfError, PdfOptions}; use super::paint::{append_path, fill, grown, norm, ok, paint_fill, pdf_line_cap, rect_path, shape_effects, shape_path}; use super::picture::{Vector, build_vector, load_image, paint_of, vector_picture};@@ -341,7 +341,7 @@ impl Exporter<'_> { let end = ends.get(i).copied().unwrap_or(text.len()); let start = g.cluster.min(end); KrillaGlyph::new(- GlyphId::new(g.gid),+ glyph_id(g.gid), g.x_advance as f32 / upem, g.x_offset as f32 / upem, g.y_offset as f32 / upem,@@ -120,7 +120,7 @@ impl Exporter<'_> { Some((_, nx, _, _)) => (nx - x) / size, None => face.advance(*g) as f32 / upem, };- KrillaGlyph::new(GlyphId::new(*g), adv, 0.0, (y0 - y) / size, 0.0, r.clone(), None)+ KrillaGlyph::new(glyph_id(*g), adv, 0.0, (y0 - y) / size, 0.0, r.clone(), None) }) .collect(); let pushed = italic;@@ -208,7 +208,7 @@ impl Exporter<'_> { None => g.x_advance as f32 / upem, }; let dy = lay.glyphs.get(i).map_or(0.0, |p| ((lay.baseline - p.1) / lay.em) as f32);- KrillaGlyph::new(GlyphId::new(g.gid), adv, 0.0, dy, 0.0, start..end, None)+ KrillaGlyph::new(glyph_id(g.gid), adv, 0.0, dy, 0.0, start..end, None) }) .collect(); let x0 = lay.glyphs.first().map_or(0.0, |p| p.0) as f32;@@ -1,6 +1,14 @@ use super::paint::append_path; use super::*; +/// A shaped glyph id as krilla takes it. Shaping gives 32-bit ids (a format-12 cmap can map to+/// any), but krilla's font subsetting takes 16-bit ones and panics on a larger id (#3674): one+/// that doesn't fit is written as .notdef (glyph 0), keeping its place. Its own character doesn't+/// extract, since krilla maps no text to .notdef.+pub(crate) fn glyph_id(g: u32) -> GlyphId {+ GlyphId::new(u16::try_from(g).map_or(0, u32::from))+}+ /// The outlines of `glyphs` (glyph id, pen position) at `size`, as one path in page coordinates. pub(crate) fn glyph_outlines(face: &FaceRef, size: f32, glyphs: &[(u32, f32, f32)]) -> Option<Path> { let db = wordcraft_fonts::FontDb::global();@@ -773,6 +773,32 @@ fn unsubsettable_font_is_drawn_as_outlines() { assert!(paths > 0, "its glyphs are drawn as outlines"); } +/// #3674: shaping gives 32-bit glyph ids (a format-12 cmap can map a character to any of them),+/// but krilla takes 16-bit ones and panics on a larger id. That failed the export, and on wasm+/// (`panic = "abort"`) lost the tab. A glyph id above 65535 is now written as .notdef.+#[test]+fn glyph_id_above_u16_exports() {+ let mut d = Document::new();+ d.body = vec![para_block(Paragraph::with_text("Wide glyph", CharProps { font: Some("Source Sans 3".into()), ..Default::default() }))];+ let opts = PdfOptions { tagged: false, ..Default::default() };+ let mut lay = layout(&d, &mut LayoutCache::new(), &LayoutOptions::default());+ let (want, _) = count_glyphs_and_paths(&export_layout(&d, &lay, &opts).unwrap());+ assert!(want > 0);+ let mut wide = 0;+ for item in lay.pages.iter_mut().flat_map(|p| p.items.iter_mut()) {+ if let Placed::Lines { para, .. } = item+ && let Some(g) = Arc::make_mut(para).glyphs.first_mut()+ {+ g.gid = 70_000;+ wide += 1;+ }+ }+ assert_eq!(wide, 1, "one glyph id is out of range");+ let bytes = export_layout(&d, &lay, &opts).expect("a glyph id above 65535 must not fail the export");+ let (glyphs, _) = count_glyphs_and_paths(&bytes);+ assert_eq!(glyphs, want, "every glyph is still drawn, the wide one as .notdef");+}+ #[test] fn flat_lines_in_a_group_are_drawn() { // A horizontal divider inside a group has no height (`a:ext cy="0"`): it used to be left out