Harden STEP, IGES and WMF readers against crafted files

A truncated or hostile file could crash, hang or exhaust memory while
being read. STEP: a last instance without its semicolon no longer runs
the cursor past the end, the complex B-spline forms read their parts
with bounds checks, a knot multiplicity is capped at degree + 1,
parameter lists nest at most 64 deep, and wrapped curves and
conversion-based units follow at most a fixed number of hops so
self-references cannot loop. IGES: spline curve and surface sizes, hole
and boundary counts and list indices are checked against the
parameters the entity actually has, using checked arithmetic;
transformation matrix chains and nested composite curves are cut after
a few links. IGES colour 1 (black) now maps to colour 7 so it shows on a
dark background. WMF: a record whose size claims more than the file has
left is clamped to the remaining bytes. Doc comments describe the
behaviour instead of naming an outside program.

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

src/io/iges_import.rs

@@ -10,6 +10,7 @@ use kernel::brep::{Circle3, Curve3, EdgeKey, Line3, Surface, VertexKey};
 use kernel::space::{NurbsCurve3, NurbsSurface3, Plane};
 
 use super::brep_build::{Builder, FaceSpec};
+use super::step_import::compose;
 
 /// One parameter: a number, or a Hollerith string.
 #[derive(Debug, Clone)]
@@ -44,7 +45,7 @@ fn parameters(text: &str, delimiter: char, terminator: char) -> Vec<P> {
         if i > start && matches!(chars.get(i), Some('H' | 'h')) {
             let count: usize = chars[start..i].iter().collect::<String>().parse().unwrap_or(0);
             let body: String = chars.iter().skip(i + 1).take(count).collect();
-            i += 1 + count;
+            i = i.saturating_add(1).saturating_add(count);
             out.push(P::Str(body));
         } else {
             i = start;
@@ -87,7 +88,13 @@ struct Doc {
 }
 
 /// A 3×4 matrix applied as `R·p + T`.
-type Matrix = [[f64; 4]; 3];
+type Matrix = super::step_import::Placement;
+
+/// A transformation matrix entity's (124) matrix.
+fn matrix_of(entity: &Entity) -> Matrix {
+    let v = |k: usize| entity.params.get(k).map_or(0.0, P::num);
+    [[v(1), v(2), v(3), v(4)], [v(5), v(6), v(7), v(8)], [v(9), v(10), v(11), v(12)]]
+}
 
 impl Doc {
     fn get(&self, pointer: f64) -> Option<&Entity> {
@@ -99,7 +106,8 @@ impl Doc {
     fn color(&self, number: i64) -> codec::types::Color {
         use codec::types::Color;
         let rgb = match number {
-            1 => [0, 0, 0],
+            // Black draws as colour 7, so it shows on a dark background too.
+            1 => return Color::Index(7),
             2 => [255, 0, 0],
             3 => [0, 255, 0],
             4 => [0, 0, 255],
@@ -127,20 +135,22 @@ impl Doc {
         }
     }
 
+    /// A transformation matrix, with the matrices it chains through its own
+    /// transformation pointer applied after it. The chain is cut after a
+    /// few links, so matrices that point at each other cannot loop.
     fn matrix(&self, pointer: usize) -> Option<Matrix> {
-        let entity = self.entities.get(&pointer).filter(|e| e.kind == 124)?;
+        let mut entity = self.entities.get(&pointer).filter(|e| e.kind == 124)?;
-        let v = |k: usize| entity.params.get(k).map_or(0.0, P::num);
+        let mut m = matrix_of(entity);
-        let mut m = [[v(1), v(2), v(3), v(4)], [v(5), v(6), v(7), v(8)], [v(9), v(10), v(11), v(12)]];
+        for _ in 0..16 {
-        // Matrices chain through their own transformation pointer.
+            let Some(outer) = (entity.transform != 0)
-        if let Some(outer) = (entity.transform != 0).then(|| self.matrix(entity.transform)).flatten() {
+                .then(|| self.entities.get(&entity.transform))
-            let mut product = [[0.0; 4]; 3];
+                .flatten()
-            for r in 0..3 {
+                .filter(|e| e.kind == 124)
-                for c in 0..4 {
+            else {
-                    product[r][c] = (0..3).map(|k| outer[r][k] * m[k][c]).sum::<f64>()
+                break;
-                        + if c == 3 { outer[r][3] } else { 0.0 };
+            };
-                }
+            m = compose(&matrix_of(outer), &m);
-            }
+            entity = outer;
-            m = product;
         }
         Some(m)
     }
@@ -194,12 +204,17 @@ impl Doc {
                 })
             }
             126 => {
+                // The sizes come from the file: refuse ones its parameters
+                // cannot hold.
                 let k = v(1) as usize;
                 let degree = v(2) as usize;
-                let knots = (0..k + degree + 2).map(|n| v(7 + n)).collect();
+                let w0 = k.checked_add(degree)?.checked_add(9)?;
-                let w0 = 7 + k + degree + 2;
+                let c0 = w0.checked_add(k)?.checked_add(1)?;
+                if c0.checked_add(k.checked_add(1)?.checked_mul(3)?)? > p.len() {
+                    return None;
+                }
+                let knots = (7..w0).map(v).collect();
                 let weights = (0..=k).map(|n| v(w0 + n)).collect();
-                let c0 = w0 + k + 1;
                 let points = (0..=k)
                     .map(|n| place([v(c0 + 3 * n), v(c0 + 3 * n + 1), v(c0 + 3 * n + 2)]))
                     .collect();
@@ -211,13 +226,22 @@ impl Doc {
 
     /// A boundary curve as its pieces — each a curve and the points it runs
     /// from and to — composite curves taken apart.
+    /// Composite curves nest only a few deep, so a deeper (or circular) one
+    /// is refused.
     fn pieces(&self, pointer: f64) -> Option<Vec<(Curve3, [f64; 3], [f64; 3])>> {
+        self.pieces_at(pointer, 0)
+    }
+
+    fn pieces_at(&self, pointer: f64, depth: usize) -> Option<Vec<(Curve3, [f64; 3], [f64; 3])>> {
         let entity = self.get(pointer)?;
         if entity.kind == 102 {
+            if depth >= 8 {
+                return None;
+            }
             let count = entity.params.get(1).map_or(0.0, P::num) as usize;
             let mut out = Vec::new();
             for k in 0..count {
-                out.extend(self.pieces(entity.params.get(2 + k)?.num())?);
+                out.extend(self.pieces_at(entity.params.get(2 + k)?.num(), depth + 1)?);
             }
             return Some(out);
         }
@@ -245,8 +269,8 @@ impl Doc {
             142 => self.pieces(v(4)),
             141 => {
                 let mut out = Vec::new();
-                let mut at = 5;
+                let mut at: usize = 5;
-                for _ in 0..v(4) as usize {
+                for _ in 0..(v(4) as usize).min(entity.params.len()) {
                     let mut pieces = self.pieces(v(at))?;
                     if v(at + 1) == 2.0 {
                         pieces.reverse();
@@ -255,7 +279,7 @@ impl Doc {
                         }
                     }
                     out.extend(pieces);
-                    at += 3 + v(at + 2) as usize;
+                    at = at.saturating_add(3).saturating_add(v(at + 2) as usize);
                 }
                 Some(out)
             }
@@ -271,10 +295,16 @@ impl Doc {
         let v = |k: usize| entity.params.get(k).map_or(0.0, P::num);
         let place = self.place(entity);
         let (k1, k2, m1, m2) = (v(1) as usize, v(2) as usize, v(3) as usize, v(4) as usize);
+        // The sizes come from the file: refuse ones its parameters cannot
+        // hold.
         let s0 = 10;
-        let t0 = s0 + k1 + m1 + 2;
+        let t0 = k1.checked_add(m1)?.checked_add(s0 + 2)?;
-        let w0 = t0 + k2 + m2 + 2;
+        let w0 = t0.checked_add(k2)?.checked_add(m2)?.checked_add(2)?;
-        let c0 = w0 + (k1 + 1) * (k2 + 1);
+        let count = k1.checked_add(1)?.checked_mul(k2.checked_add(1)?)?;
+        let c0 = w0.checked_add(count)?;
+        if c0.checked_add(count.checked_mul(3)?)? > entity.params.len() {
+            return None;
+        }
         let u_knots = (s0..t0).map(v).collect();
         let v_knots = (t0..w0).map(v).collect();
         // Points run with the first index fastest.
@@ -316,7 +346,7 @@ impl Assembly<'_> {
             return Some(v);
         }
         let entity = self.doc.get(list).filter(|e| e.kind == 502)?;
-        let n = 2 + 3 * (key.1.checked_sub(1)?);
+        let n = key.1.checked_sub(1)?.checked_mul(3)?.checked_add(2)?.min(entity.params.len());
         let p = |k: usize| entity.params.get(k).map(P::num);
         let point = self.doc.place(entity)([p(n)?, p(n + 1)?, p(n + 2)?]);
         let v = self.build.vertex(point);
@@ -330,7 +360,7 @@ impl Assembly<'_> {
             return Some(e);
         }
         let entity = self.doc.get(list).filter(|e| e.kind == 504)?;
-        let n = 2 + 5 * key.1.checked_sub(1)?;
+        let n = key.1.checked_sub(1)?.checked_mul(5)?.checked_add(2)?.min(entity.params.len());
         let p = |k: usize| entity.params.get(k).map(P::num);
         let curve = self.doc.curve(p(n)?)?;
         let start = self.vertex(p(n + 1)?, p(n + 2)?)?;
@@ -417,16 +447,11 @@ impl Assembly<'_> {
     /// A trimmed (144) or bounded (143) surface as a one-face sheet.
     fn sheet(&mut self, entity: &Entity) -> Option<()> {
         let v = |k: usize| entity.params.get(k).map_or(0.0, P::num);
+        // A count can be no more than the parameters there are.
+        let count = (v(3) as usize).min(entity.params.len());
         let (surface_at, outer, holes): (f64, Option<f64>, Vec<f64>) = match entity.kind {
-            144 => (
+            144 => (v(1), (v(2) != 0.0).then(|| v(4)), (0..count).map(|k| v(5 + k)).collect()),
-                v(1),
+            143 => (v(2), (count > 0).then(|| v(4)), (1..count).map(|k| v(4 + k)).collect()),
-                (v(2) != 0.0).then(|| v(4)),
-                (0..v(3) as usize).map(|k| v(5 + k)).collect(),
-            ),
-            143 => {
-                let count = v(3) as usize;
-                (v(2), (count > 0).then(|| v(4)), (1..count).map(|k| v(4 + k)).collect())
-            }
             _ => return None,
         };
         let Surface::Nurbs(nurbs) = self.doc.surface(surface_at)? else { return None };
@@ -463,7 +488,7 @@ impl Assembly<'_> {
             if kind == 0.0 {
                 ring.push((self.edge(list, index)?, agrees != 0.0));
             }
-            at += 5 + 2 * pcurves as usize;
+            at = at.saturating_add(5).saturating_add((pcurves as usize).saturating_mul(2));
         }
         Some(ring)
     }
@@ -579,9 +604,10 @@ pub fn read(bytes: &[u8], drawing_mm: Option<f64>) -> Option<IgesModel> {
         let Some(number) = first.get(73..).and_then(|n| n.trim().parse::<usize>().ok()) else {
             continue;
         };
-        let start = field(first, 1) as usize;
+        // An entry spans at most every parameter line there is.
-        let count = field(second, 3).max(1) as usize;
+        let start = field(first, 1).max(0) as usize;
-        let data: String = (start..start + count)
+        let count = (field(second, 3).max(1) as usize).min(parameter_lines.len());
+        let data: String = (start..start.saturating_add(count))
             .filter_map(|n| parameter_lines.get(&n))
             .filter_map(|l| l.get(..64))
             .collect();
@@ -635,7 +661,7 @@ pub fn read(bytes: &[u8], drawing_mm: Option<f64>) -> Option<IgesModel> {
             }
         }
     }
-    // The solids come in last first, as the reference reader places them.
+    // The solids are listed last first.
     bodies.reverse();
     let mut numbers: Vec<usize> = doc
         .entities

src/io/step_import.rs

@@ -61,6 +61,8 @@ type Instance = Vec<(String, Vec<P>)>;
 struct Cursor<'a> {
     s: &'a [u8],
     i: usize,
+    /// How many parameter lists deep the cursor is.
+    depth: usize,
 }
 
 impl Cursor<'_> {
@@ -69,7 +71,7 @@ impl Cursor<'_> {
             while self.s.get(self.i).is_some_and(|c| c.is_ascii_whitespace()) {
                 self.i += 1;
             }
-            if self.s[self.i..].starts_with(b"/*") {
+            if self.s.get(self.i..).is_some_and(|rest| rest.starts_with(b"/*")) {
                 match self.s[self.i + 2..].windows(2).position(|w| w == b"*/") {
                     Some(end) => self.i += end + 4,
                     None => self.i = self.s.len(),
@@ -152,7 +154,18 @@ impl Cursor<'_> {
             }
         })
     }
+    /// A parameter list; lists nested past any real file's depth are
+    /// refused rather than read on until the stack runs out.
     fn params(&mut self) -> Option<Vec<P>> {
+        if self.depth >= 64 {
+            return None;
+        }
+        self.depth += 1;
+        let items = self.list();
+        self.depth -= 1;
+        items
+    }
+    fn list(&mut self) -> Option<Vec<P>> {
         self.eat(b'(')?;
         let mut items = Vec::new();
         if self.eat(b')').is_some() {
@@ -302,11 +315,12 @@ impl Doc {
         Plane::orthonormal(origin, x, z)
     }
 
-    /// Knots expanded by their multiplicities.
+    /// Knots expanded by their multiplicities. No knot repeats more than
-    fn knots(mults: &P, knots: &P) -> Option<Vec<f64>> {
+    /// `degree + 1` times, so a bogus multiplicity cannot blow up the list.
+    fn knots(mults: &P, knots: &P, degree: usize) -> Option<Vec<f64>> {
         let mut out = Vec::new();
         for (m, k) in mults.list().iter().zip(knots.list()) {
-            for _ in 0..m.num()? as usize {
+            for _ in 0..(m.num()? as usize).min(degree.saturating_add(1)) {
                 out.push(k.num()?);
             }
         }
@@ -314,7 +328,20 @@ impl Doc {
     }
 
     fn curve(&self, p: &P) -> Option<Curve3> {
-        let id = p.reference()?;
+        let mut id = p.reference()?;
+        // Curves that wrap a basis curve are followed to it; the hop limit
+        // stops curves that wrap each other in a loop.
+        let mut hops = 0;
+        while matches!(
+            self.kind(id),
+            "SURFACE_CURVE" | "SEAM_CURVE" | "INTERSECTION_CURVE" | "TRIMMED_CURVE"
+        ) {
+            hops += 1;
+            if hops > 32 {
+                return None;
+            }
+            id = self.simple(id)?.1.get(1)?.reference()?;
+        }
         let kind = self.kind(id);
         if kind == "B_SPLINE_CURVE_WITH_KNOTS" || self.part(id, "B_SPLINE_CURVE").is_some() {
             return self.spline_curve(id).map(Curve3::Nurbs);
@@ -340,9 +367,6 @@ impl Doc {
                 major_radius: params.get(2)?.num()?,
                 minor_radius: params.get(3)?.num()?,
             }),
-            "SURFACE_CURVE" | "SEAM_CURVE" | "INTERSECTION_CURVE" | "TRIMMED_CURVE" => {
-                self.curve(params.get(1)?)?
-            }
             "POLYLINE" => {
                 let points: Option<Vec<_>> =
                     params.get(1)?.list().iter().map(|q| self.point(q)).collect();
@@ -362,13 +386,13 @@ impl Doc {
                 let base = self.part(id, "B_SPLINE_CURVE")?;
                 let with = self.part(id, "B_SPLINE_CURVE_WITH_KNOTS")?;
                 let weights = self.part(id, "RATIONAL_B_SPLINE_CURVE").and_then(|w| w.first());
-                (&base[0], &base[1], &with[0], &with[1], weights)
+                (base.first()?, base.get(1)?, with.first()?, with.get(1)?, weights)
             };
         let points: Option<Vec<_>> = points.list().iter().map(|q| self.point(q)).collect();
         let weights: Option<Vec<f64>> = weights.map(|w| w.list().iter().filter_map(P::num).collect());
         let degree = degree.num()? as usize;
         let points = points?;
-        let knots = Self::knots(mults, knots)?;
+        let knots = Self::knots(mults, knots, degree)?;
         match &weights {
             Some(w) => NurbsCurve3::new_strict(degree, points.clone(), knots.clone(), w.clone())
                 .or_else(|| NurbsCurve3::new(degree, points, knots, weights.clone())),
@@ -420,7 +444,16 @@ impl Doc {
             let base = self.part(id, "B_SPLINE_SURFACE")?;
             let with = self.part(id, "B_SPLINE_SURFACE_WITH_KNOTS")?;
             let weights = self.part(id, "RATIONAL_B_SPLINE_SURFACE").and_then(|w| w.first());
-            (&base[0], &base[1], &base[2], &with[0], &with[1], &with[2], &with[3], weights)
+            (
+                base.first()?,
+                base.get(1)?,
+                base.get(2)?,
+                with.first()?,
+                with.get(1)?,
+                with.get(2)?,
+                with.get(3)?,
+                weights,
+            )
         };
         let rows: Option<Vec<Vec<[f64; 3]>>> = points
             .list()
@@ -433,14 +466,8 @@ impl Doc {
                 .map(|row| row.list().iter().filter_map(P::num).collect())
                 .collect()
         });
-        NurbsSurface3::new(
+        let (ud, vd) = (ud.num()? as usize, vd.num()? as usize);
-            ud.num()? as usize,
+        NurbsSurface3::new(ud, vd, rows?, Self::knots(um, uk, ud)?, Self::knots(vm, vk, vd)?, weights)
-            vd.num()? as usize,
-            rows?,
-            Self::knots(um, uk)?,
-            Self::knots(vm, vk)?,
-            weights,
-        )
     }
 }
 
@@ -595,8 +622,18 @@ pub struct StepModel {
 }
 
 /// A named unit's size: millimetres for a length, radians for an angle.
-fn unit_size(doc: &Doc, id: u64) -> Option<f64> {
+/// A conversion-based unit is a multiple of another unit; the hop limit
-    if let Some(si) = doc.part(id, "SI_UNIT") {
+/// stops units defined in terms of each other.
+fn unit_size(doc: &Doc, mut id: u64) -> Option<f64> {
+    let mut factor = 1.0;
+    for _ in 0..16 {
+        let Some(si) = doc.part(id, "SI_UNIT") else {
+            let conversion = doc.part(id, "CONVERSION_BASED_UNIT")?;
+            let (_, measure) = doc.simple(conversion.get(1)?.reference()?)?;
+            factor *= measure.first()?.num()?;
+            id = measure.get(1)?.reference()?;
+            continue;
+        };
         let prefix = match si.first() {
             Some(P::Enum(prefix)) => match prefix.as_str() {
                 "MILLI" => 1e-3,
@@ -608,14 +645,15 @@ fn unit_size(doc: &Doc, id: u64) -> Option<f64> {
             },
             _ => 1.0,
         };
-        return Some(match si.get(1) {
+        return Some(
-            Some(P::Enum(name)) if name == "METRE" => prefix * 1000.0,
+            factor
-            _ => prefix,
+                * match si.get(1) {
-        });
+                    Some(P::Enum(name)) if name == "METRE" => prefix * 1000.0,
+                    _ => prefix,
+                },
+        );
     }
-    let conversion = doc.part(id, "CONVERSION_BASED_UNIT")?;
+    None
-    let (_, measure) = doc.simple(conversion.get(1)?.reference()?)?;
-    Some(measure.first()?.num()? * unit_size(doc, measure.get(1)?.reference()?)?)
 }
 
 /// The bodies of one B-rep item: a solid, a solid with voids, or each shell
@@ -680,7 +718,7 @@ impl Doc {
 }
 
 /// `a·b`.
-fn compose(a: &Placement, b: &Placement) -> Placement {
+pub(super) fn compose(a: &Placement, b: &Placement) -> Placement {
     let mut out = [[0.0; 4]; 3];
     for r in 0..3 {
         for c in 0..4 {
@@ -711,6 +749,7 @@ pub fn read(bytes: &[u8]) -> Option<StepModel> {
     let mut cursor = Cursor {
         s: &text.as_bytes()[data..],
         i: 0,
+        depth: 0,
     };
     let mut instances = HashMap::new();
     while cursor.peek() == Some(b'#') {
@@ -723,7 +762,8 @@ pub fn read(bytes: &[u8]) -> Option<StepModel> {
                 while cursor.s.get(cursor.i).is_some_and(|c| *c != b';') {
                     cursor.i += 1;
                 }
-                cursor.i += 1;
+                // Past the `;`, or at the end of a truncated file.
+                cursor.i = (cursor.i + 1).min(cursor.s.len());
             }
         }
     }

src/io/wmf_import.rs

@@ -209,8 +209,8 @@ fn radial_angle(bounds: [f64; 4], x: f64, y: f64) -> Option<f64> {
     Some(if degrees == 0.0 { 360.0 } else { degrees })
 }
 
-/// An arc as the reference reader plays it: points round the ellipse's
+/// An arc's points: round the ellipse's whole-unit centre and radii at
-/// whole-unit centre and radii at whole multiples of 1/99 turn, rounded
+/// whole multiples of 1/99 turn, rounded
 /// down to whole units, from the start angle up to the end angle — past
 /// 360 degrees back from zero when the end comes first. A start inside the
 /// ellipse starts at zero; an end inside it runs on to a whole turn; with
@@ -305,9 +305,11 @@ pub fn read(bytes: &[u8], fit: [f64; 2]) -> Option<Picture> {
         if function == 0 || size < 3 {
             break;
         }
-        let params: Vec<u16> = (0..size - 3).filter_map(|k| word(at + 6 + 2 * k)).collect();
+        // A record claims no more words than the file has left.
+        let words = (size - 3).min((bytes.len() - (at + 6)) / 2);
+        let params: Vec<u16> = (0..words).filter_map(|k| word(at + 6 + 2 * k)).collect();
         records.push((function, params));
-        at += 2 * size;
+        at = at.saturating_add(size.saturating_mul(2));
     }
 
     // The window first: where the picture is and how big.