From 4c82749b08575cec1e6a0fe2e2bca1ef157151e7 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Fri, 23 Oct 2026 22:43:42 -0400 Subject: [PATCH] Security: Affinity import and brush expansion bounds (#3887) * Audit: Affinity import (spot colour chain, zero-width records, corner radii) * Audit: Affinity layer names are cut at 1024 characters * Audit: brush expansion bounds (masks, library parsing, flattening), pressure profile lookup * Brush: reach a node's mask through its extras --- crates/affinity/src/geometry.rs | 8 +++++++- crates/affinity/src/model.rs | 6 +++++- crates/affinity/src/paint.rs | 13 ++++++++++++- crates/affinity/src/stream.rs | 4 ++++ crates/brush/src/colorize.rs | 22 ++++++++++++++++++++++ crates/brush/src/lib.rs | 4 ++++ crates/brush/src/track.rs | 15 ++++++++++++++- crates/doc/src/appearance.rs | 12 ++++++------ 8 files changed, 74 insertions(+), 10 deletions(-) diff --git a/crates/affinity/src/geometry.rs b/crates/affinity/src/geometry.rs index 5d74638ad..8e84e4807 100644 --- a/crates/affinity/src/geometry.rs +++ b/crates/affinity/src/geometry.rs @@ -215,7 +215,13 @@ fn anchors(records: &[Value]) -> Option> { fn subpath(anchors: &[Anchor], closed: bool, radii: Option<&Vec<(usize, f64)>>) -> Option { let first = anchors.first()?; if let Some(radii) = radii.filter(|r| !r.is_empty()) { - let corner = |a: &Anchor| radii.iter().filter(|(rec, _)| *rec == a.record).map(|(_, r)| *r).fold(0.0, f64::max); + // The largest radius of each record, once (not a scan of all of them for each anchor). + let mut largest: std::collections::HashMap = std::collections::HashMap::new(); + for (rec, r) in radii { + let e = largest.entry(*rec).or_insert(0.0); + *e = e.max(*r); + } + let corner = |a: &Anchor| largest.get(&a.record).copied().unwrap_or(0.0); let polyline = anchors.iter().all(|a| a.inh.is_none_or(|h| h == a.p) && a.out.is_none_or(|h| h == a.p)); if polyline { let mut pts: Vec<(Point, f64)> = anchors.iter().map(|a| (a.p, corner(a))).collect(); diff --git a/crates/affinity/src/model.rs b/crates/affinity/src/model.rs index 5a1feaab4..7f10f8823 100644 --- a/crates/affinity/src/model.rs +++ b/crates/affinity/src/model.rs @@ -517,6 +517,9 @@ struct ReadAt { /// Read a document whose spreads are placed by `base` (for embedded documents, nested `depth` /// deep). What it uses is taken from `budget`, also when it fails part of the way. +/// The most characters of a layer's name kept. +const MAX_NAME_CHARS: usize = 1024; + fn read_at(bytes: &[u8], limits: Limits, budget: &mut Budget, base: Affine, depth: usize) -> ReadAt { let mut archive = match Archive::open(bytes, limits) { Ok(a) => a, @@ -692,7 +695,8 @@ impl<'s, 'a, 'b> Reader<'s, 'a, 'b> { let local = s.floats::<6>(id, b"Xfrm").map(Affine::from_xfrm).unwrap_or(Affine::IDENTITY); let world = local.then(parent); let class = s.class(id).unwrap_or(Tag(0)); - let name = s.str(id, b"Desc").unwrap_or_default().to_string(); + // A layer name is short; a long one is cut, as every visit of a shared object keeps a copy. + let name: String = s.str(id, b"Desc").unwrap_or_default().chars().take(MAX_NAME_CHARS).collect(); let visible = s.bool(id, b"Visi").unwrap_or(true); let locked = s.bool(id, b"Edtb").is_some_and(|e| !e); let opacity = s.f64(id, b"Opac").map_or(1.0, |o| o.clamp(0.0, 1.0)); diff --git a/crates/affinity/src/paint.rs b/crates/affinity/src/paint.rs index 3eea67795..9324b0335 100644 --- a/crates/affinity/src/paint.rs +++ b/crates/affinity/src/paint.rs @@ -139,6 +139,14 @@ fn floats(v: &Value) -> Option> { /// A colour object (`RGBA`, `HSLA`, `CMYK`, `GRAY`, `LABA`, `Pant`). pub(crate) fn color(r: &mut Reader, id: ObjId) -> Option { + color_within(r, id, 0) +} + +/// The most spot colours that stand for one another followed. +const MAX_SPOT_CHAIN: u32 = 8; + +/// [`color`], `depth` spot colours deep: one that stands for itself (or a ring of them) ends here. +fn color_within(r: &mut Reader, id: ObjId, depth: u32) -> Option { let s = r.s; let class = s.class(id)?; let col = || -> Option> { @@ -170,7 +178,10 @@ pub(crate) fn color(r: &mut Reader, id: ObjId) -> Option { } b"Pant" => { r.warn("spot colours (imported as their RGB equivalent)"); - return s.obj(id, b"srgb").and_then(|c| color(r, c)).or_else(|| s.obj(id, b"base").and_then(|c| color(r, c))); + if depth >= MAX_SPOT_CHAIN { + return None; + } + return s.obj(id, b"srgb").and_then(|c| color_within(r, c, depth + 1)).or_else(|| s.obj(id, b"base").and_then(|c| color_within(r, c, depth + 1))); } b"RegC" => Color::Cmyk { c: 1.0, m: 1.0, y: 1.0, k: 1.0, a: 1.0 }, _ => { diff --git a/crates/affinity/src/stream.rs b/crates/affinity/src/stream.rs index a755747c4..7a3010251 100644 --- a/crates/affinity/src/stream.rs +++ b/crates/affinity/src/stream.rs @@ -303,6 +303,10 @@ impl Parser<'_> { 0x2c => { let n = usize::try_from(self.c.u32()?).map_err(|_| Error::Malformed("array length"))?; let size = usize::from(self.c.u16()?); + // Records of no bytes take nothing from the data, whatever their number. + if n > 0 && size == 0 { + return Err(Error::Malformed("array of records without bytes")); + } if n.checked_mul(size).is_none_or(|b| b > self.c.remaining()) { return Err(Error::Malformed("array longer than its data")); } diff --git a/crates/brush/src/colorize.rs b/crates/brush/src/colorize.rs index 2890790da..80d302b66 100644 --- a/crates/brush/src/colorize.rs +++ b/crates/brush/src/colorize.rs @@ -77,10 +77,32 @@ pub fn colorize(n: &mut Node, mode: Colorization, stroke: &Paint) { walk(Arc::make_mut(c), mode, key); } } + // A mask's art is drawn too: a brush on its strokes would run this brush again. Its colours + // are the mask's values, so they stay. + if let Some(m) = n.extras_if_any().and_then(|e| e.mask.as_mut()) { + strip_brushes(Arc::make_mut(&mut m.art)); + } } walk(n, mode, key); } +/// Strip the brushes from every stroke under `n`, its masks' art included. +fn strip_brushes(n: &mut Node) { + for it in &mut n.appearance.items { + if let AppearanceItem::Stroke(s) = it { + s.brush = None; + } + } + if let Some(ch) = n.children_mut() { + for c in ch.iter_mut() { + strip_brushes(Arc::make_mut(c)); + } + } + if let Some(m) = n.extras_if_any().and_then(|e| e.mask.as_mut()) { + strip_brushes(Arc::make_mut(&mut m.art)); + } +} + /// Mix every solid paint under `n` towards `color` by `amount` (0..1) — the Symbol Stainer. pub fn tint_node(n: &mut Node, color: &Color, amount: f32) { let amount = amount.clamp(0.0, 1.0); diff --git a/crates/brush/src/lib.rs b/crates/brush/src/lib.rs index 24f4e7f5a..1f7af6e60 100644 --- a/crates/brush/src/lib.rs +++ b/crates/brush/src/lib.rs @@ -689,6 +689,10 @@ fn leave_out_brushed(doc: &Document, nodes: &mut Vec, known: &mut Known) { /// with the stroke's opacity folded into the piece opacity. `None` if nothing is brushed. pub fn node_pieces(doc: &Document, n: &Node) -> Option> { let bp = node_bezpath(n)?; + // The library is parsed for a stroke that names a brush, not for every path. + if !n.appearance.items.iter().any(|i| matches!(i, AppearanceItem::Stroke(st) if st.brush.is_some())) { + return None; + } let lib = library(doc); let mut out = vec![]; let mut any = false; diff --git a/crates/brush/src/track.rs b/crates/brush/src/track.rs index e02bcf48d..9e651a670 100644 --- a/crates/brush/src/track.rs +++ b/crates/brush/src/track.rs @@ -156,6 +156,9 @@ impl Track { } } +/// The most flattened elements of a path kept. +const MAX_FLATTENED: usize = 4_000_000; + /// Flatten `bp` into tracks (one per subpath) with `tol` accuracy. pub fn tracks(bp: &BezPath, tol: f64) -> Vec { let mut out = vec![]; @@ -165,8 +168,18 @@ pub fn tracks(bp: &BezPath, tol: f64) -> Vec { out.push(t); } }; + // Far outside any drawing a curve flattens to millions of points; past a few million, the + // rest are not kept. + let b = bp.control_box(); + if !(b.x0.abs().max(b.x1.abs()).max(b.y0.abs()).max(b.y1.abs()) <= 1e9) { + return out; + } let mut els = vec![]; - kurbo::flatten(bp.iter(), tol.max(1e-4), |el| els.push(el)); + kurbo::flatten(bp.iter(), tol.max(1e-4), |el| { + if els.len() < MAX_FLATTENED { + els.push(el); + } + }); for el in els { match el { PathEl::MoveTo(p) => { diff --git a/crates/doc/src/appearance.rs b/crates/doc/src/appearance.rs index b336ff152..9895cc7f9 100644 --- a/crates/doc/src/appearance.rs +++ b/crates/doc/src/appearance.rs @@ -314,15 +314,15 @@ impl PressureProfile { pub fn at(&self, t: f64) -> f64 { let p = &self.points; let (Some(first), Some(last)) = (p.first(), p.last()) else { return Self::MID }; + // (Positions go forward; a profile read from a file is searched as if they do, in log time.) let v = if t <= first.0 { first.1 } else { - p.windows(2) - .find_map(|w| match w { - [a, b] if t <= b.0 => Some(a.1 + (b.1 - a.1) * ((t - a.0) / (b.0 - a.0).max(1e-9)).clamp(0.0, 1.0)), - _ => None, - }) - .unwrap_or(last.1) + let i = p.partition_point(|q| q.0 < t); + match (i.checked_sub(1).and_then(|j| p.get(j)), p.get(i)) { + (Some(a), Some(b)) => a.1 + (b.1 - a.1) * ((t - a.0) / (b.0 - a.0).max(1e-9)).clamp(0.0, 1.0), + _ => last.1, + } }; if v.is_finite() { v.clamp(0.0, 1.0) } else { Self::MID } }