Security: document model bounds (id hashing, page ranges, overprint migration, auto slices, nested Repeat bounds) (#3888)

* Audit: document model bounds (id hashing, ranges, overprint migration, auto slices)

* Audit: nested Repeat bounds are measured once per level

* fix(slices): bound auto_slices accumulation strictly by MAX_AUTO_SLICES

* chore(text): use is_some_and instead of unnecessary map_or (clippy)

* Refuse automatic slice layout past a cut-rectangle bound

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>

crates/doc/src/id_index.rs

@@ -20,8 +20,9 @@ use std::sync::{Arc, RwLock};
 
 use crate::{Node, NodeId};
 
-/// Hasher for node ids: one multiply. Ids are small counters, not attacker-chosen keys, and the
-/// default SipHash dominated the cost of building the cache.
+/// Hasher for node ids: one multiply, by a number chosen at random once per run (a document's ids
+/// are its author's, and a fixed multiplier lets them be picked to collide). The default SipHash
+/// dominated the cost of building the cache.
 #[derive(Default)]
 pub struct IdHasher(u64);
 
@@ -32,13 +33,19 @@ impl Hasher for IdHasher {
         }
     }
     fn write_u64(&mut self, v: u64) {
-        self.0 = (self.0 ^ v).wrapping_mul(0x9e37_79b9_7f4a_7c15);
+        self.0 = (self.0 ^ v).wrapping_mul(multiplier());
     }
     fn finish(&self) -> u64 {
         self.0 ^ (self.0 >> 29)
     }
 }
 
+/// The odd multiplier of [`IdHasher`], random per run.
+fn multiplier() -> u64 {
+    static KEY: std::sync::OnceLock<u64> = std::sync::OnceLock::new();
+    *KEY.get_or_init(|| std::hash::BuildHasher::hash_one(&std::hash::RandomState::new(), 0x9e37_79b9_7f4a_7c15_u64) | 1)
+}
+
 /// A map keyed by node id with [`IdHasher`].
 pub type IdMap<V> = HashMap<NodeId, V, BuildHasherDefault<IdHasher>>;
 /// Where each node is: its parent's entry (`u32::MAX` for a top-level layer) and its index among

crates/doc/src/overprint.rs

@@ -142,8 +142,10 @@ impl Document {
         let ids: Vec<NodeId> = list.as_array().into_iter().flatten().filter_map(|v| v.as_u64()).map(NodeId).collect();
         // The old list marked spot-linked blacks too (so the spot names don't matter).
         let op = OverprintBlack { spot: true, ..Default::default() };
-        for id in ids {
-            if let Some(n) = self.node_mut(id) {
+        // One walk of the tree finds them all (a missing id costs a walk each, and a list can repeat it).
+        let paths = self.index_paths(ids);
+        for path in paths.values() {
+            if let Some(n) = self.node_mut_at(path) {
                 op.apply(n, &[]);
             }
         }

crates/doc/src/pattern.rs

@@ -632,7 +632,12 @@ impl RepeatSpec {
     /// Where the instances go, counting toward the budget the Repeats in the art of the symbols
     /// of `doc` that instances in the source show (none without a document).
     pub fn transforms(&self, doc: Option<&Document>) -> Vec<Affine> {
-        let mut v = repeat_transforms(&self.kind, self.source_bounds());
+        self.transforms_from(doc, self.source_bounds())
+    }
+    /// [`Self::transforms`] with the source's bounds (as [`Self::source_bounds`] measures them)
+    /// already at hand: measuring them again, at every level of nested Repeats, doubles the work.
+    fn transforms_from(&self, doc: Option<&Document>, bounds: Option<Rect>) -> Vec<Affine> {
+        let mut v = repeat_transforms(&self.kind, bounds);
         // Each instance draws the Repeats in the source again, so nested ones multiply (a 500 × 500
         // grid of one is 62.5 billion instances): keep the first that fit the budget.
         v.truncate(self.kept(NestedCount::new(doc).nodes(&self.source)));
@@ -681,7 +686,9 @@ impl RepeatSpec {
     /// The bounds of all instances of a source measuring `source`, each placed by its transform
     /// ([`Self::transforms`] for `doc`).
     pub(crate) fn copies_bounds(&self, source: Rect, doc: Option<&Document>) -> Option<Rect> {
-        self.transforms(doc).into_iter().fold(None, |acc, m| vectorcraft_geom::union_opt(acc, Some(m.transform_rect_bbox(source))))
+        // Without a document the source's bounds are the ones `source` is: not measured twice.
+        let transforms = if doc.is_none() { self.transforms_from(None, Some(source)) } else { self.transforms(doc) };
+        transforms.into_iter().fold(None, |acc, m| vectorcraft_geom::union_opt(acc, Some(m.transform_rect_bbox(source))))
     }
     /// Apply a document transform (source + arrangement parameters).
     pub fn transform(&mut self, a: Affine, scaling: impl Into<crate::node::Scaling>) {

crates/doc/src/range.rs

@@ -17,6 +17,7 @@ pub fn parse_range(s: &str, count: usize) -> Result<Vec<usize>, String> {
             .ok_or_else(|| format!("range `{s}`: `{t}` is not a number from 1 to {count}"))
     };
     let mut out = vec![];
+    let mut seen = std::collections::HashSet::new();
     for part in s.split(',').map(str::trim).filter(|p| !p.is_empty()) {
         let (a, b) = match part.replace('\u{2013}', "-").split_once('-') {
             Some((a, b)) if a.trim().is_empty() && b.trim().is_empty() => return Err(format!("range `{s}`: `{part}` names no number")),
@@ -30,7 +31,7 @@ pub fn parse_range(s: &str, count: usize) -> Result<Vec<usize>, String> {
             return Err(format!("range `{s}`: `{part}` runs backwards"));
         }
         for i in a - 1..b {
-            if !out.contains(&i) {
+            if seen.insert(i) {
                 out.push(i);
             }
         }

crates/doc/src/slices.rs

@@ -274,6 +274,10 @@ pub fn auto_slices(region: Rect, cut: &[Rect]) -> Vec<Rect> {
         return vec![];
     }
     let inside: Vec<Rect> = cut.iter().map(|r| region.intersect(*r)).filter(|r| valid_rect(*r)).collect();
+    // Every band scans every cut: past a bound the layout is refused, not truncated.
+    if inside.len() > MAX_AUTO_CUTS {
+        return vec![];
+    }
     let mut ys: Vec<f64> = inside.iter().flat_map(|r| [r.y0, r.y1]).chain([region.y0, region.y1]).collect();
     ys.sort_by(f64::total_cmp);
     ys.dedup_by(|a, b| (*a - *b).abs() <= EPS);
@@ -298,6 +302,9 @@ pub fn auto_slices(region: Rect, cut: &[Rect]) -> Vec<Rect> {
         // Spans continuing an open slice extend it; the others close.
         let mut next = Vec::with_capacity(free.len());
         for (a, b) in free {
+            if done.len() + open.len() + next.len() >= MAX_AUTO_SLICES {
+                break;
+            }
             match open.iter().position(|r: &Rect| (r.x0 - a).abs() <= EPS && (r.x1 - b).abs() <= EPS) {
                 Some(i) => {
                     let mut r = open.swap_remove(i);
@@ -309,7 +316,18 @@ pub fn auto_slices(region: Rect, cut: &[Rect]) -> Vec<Rect> {
         }
         done.append(&mut open);
         open = next;
+        // Crossing strips make a cell of every crossing: the layout stops, short, past a bound.
+        if done.len() + open.len() >= MAX_AUTO_SLICES {
+            break;
+        }
     }
     done.append(&mut open);
+    done.truncate(MAX_AUTO_SLICES);
     done
 }
+
+/// The most slices automatic layout makes.
+const MAX_AUTO_SLICES: usize = 10_000;
+
+/// The most cut rectangles automatic layout works through.
+const MAX_AUTO_CUTS: usize = 2_000;