Merge pull request #154 from EmDecay/fix/146-step-iges-expansions

fix: bound four unbounded expansions in STEP and IGES import

crates/kernel/src/iges_in.rs

@@ -14,6 +14,12 @@ pub const MAX_IGES_BYTES: usize = 512 << 20;
 const MAX_ENTITIES: usize = 2_000_000;
 const MAX_DEPTH: usize = 32;
 const MAX_LIST: usize = 1_000_000;
+/// Entities visited and segments produced while expanding model curves. A composite curve
+/// (102) lists its children by pointer, and nothing stops a tree whose every node names the
+/// same child twice: 30 levels under the depth cap is 2^30 leaf visits and about 160 GB of
+/// segments. The depth cap alone does not see fan-out, and a node that produces no segment
+/// trips no output bound, so the visits are counted too.
+const MAX_CURVE_WORK: usize = 4 * MAX_ENTITIES;
 
 type R<T> = Result<T, String>;
 type V3 = [f64; 3];
@@ -249,7 +255,16 @@ fn parse(text: &str) -> R<Iges> {
         des.insert(2 * k + 1, de);
     }
     let mut params = HashMap::new();
+    // Nothing stops thousands of directory entries from naming the same parameter range, and
+    // each one materialises that range as its own string and token list. Keep the total text
+    // proportional to the parameter section the file actually carries.
+    let max_lines = p_lines.len().saturating_mul(4).saturating_add(1024);
+    let mut lines_used = 0usize;
     for (seq, de) in &des {
+        lines_used = lines_used.saturating_add(de.lines.max(1));
+        if lines_used > max_lines {
+            return Err("the file's parameter data is too large".into());
+        }
         let mut s = String::new();
         for l in de.pd..de.pd.saturating_add(de.lines.max(1)) {
             // Columns 1–64 run on (a long string may continue on the next line).
@@ -368,6 +383,10 @@ struct Tr<'a> {
     /// Sewing tolerance (mm).
     tol: f64,
     surf_cache: HashMap<(usize, [u64; 12]), u64>,
+    /// Entities visited and segments produced while expanding curves (see `MAX_CURVE_WORK`).
+    curve_work: usize,
+    /// Composite curves (102) on the path being expanded.
+    composites: Vec<i64>,
 }
 
 fn xf_key(x: &Xf) -> [u64; 12] {
@@ -442,6 +461,19 @@ impl<'a> Tr<'a> {
         if depth > MAX_DEPTH {
             return Err("curves nested too deeply".into());
         }
+        self.curve_work = self.curve_work.saturating_add(1);
+        if self.curve_work > MAX_CURVE_WORK {
+            return Err("the file's curves are too complex".into());
+        }
+        let out = self.curve_parts(seq, xf, depth)?;
+        self.curve_work = self.curve_work.saturating_add(out.len());
+        if self.curve_work > MAX_CURVE_WORK {
+            return Err("a curve has too many segments".into());
+        }
+        Ok(out)
+    }
+
+    fn curve_parts(&mut self, seq: i64, xf: &Xf, depth: usize) -> R<Vec<Seg>> {
         let de = self.de(seq)?;
         let x = xf.then(&self.own_xf(seq, 0)?);
         match de.typ {
@@ -479,13 +511,23 @@ impl<'a> Tr<'a> {
                 Ok(vec![Seg::along(cid, |t| at(a0 + (a1 - a0) * t))])
             }
             102 => {
+                // A composite curve that contains itself, directly or through another, would
+                // expand until the depth cap; refusing it on the way down is clearer.
+                if self.composites.contains(&seq) {
+                    return Err(format!("entity {seq}: composite curve contains itself"));
+                }
                 let n = self.i(seq, 1)?.clamp(0, MAX_LIST as i64) as usize;
                 let mut out = Vec::new();
-                for k in 0..n {
-                    let s = self.i(seq, 2 + k)?;
-                    out.extend(self.curve(s, &x, depth + 1)?);
-                }
-                Ok(out)
+                self.composites.push(seq);
+                let r = (|| {
+                    for k in 0..n {
+                        let s = self.i(seq, 2 + k)?;
+                        out.extend(self.curve(s, &x, depth + 1)?);
+                    }
+                    Ok(out)
+                })();
+                self.composites.pop();
+                r
             }
             126 => {
                 let nb = self.nurbs_curve(seq, &x)?;
@@ -833,7 +875,9 @@ struct Sewer {
 
 impl Sewer {
     fn vertex(&mut self, p: V3) -> usize {
-        let c = |x: f64| (x / self.tol).floor() as i64;
+        // Neighbouring buckets are this key plus or minus one, so leave room for that: a
+        // coordinate near f64's range saturates the cast and the addition would overflow.
+        let c = |x: f64| (x / self.tol).floor().clamp(-9e15, 9e15) as i64;
         let key = [c(p[0]), c(p[1]), c(p[2])];
         for dx in -1..=1 {
             for dy in -1..=1 {
@@ -1008,13 +1052,23 @@ fn write_shell(o: &mut Out, sw: &Sewer, faces: &[FaceB], styles: &mut Vec<(u64,
 }
 
 /// IGES text restated as a STEP exchange (millimetres), with what could not be read.
+/// This was the one importer stage outside every panic guard, so a panic here reached the CLI
+/// and the wasm build, where nothing catches it.
 pub fn iges_to_step(text: &str) -> std::result::Result<(String, Vec<String>), String> {
+    crate::guard("IGES import", || translate(text).map_err(crate::KernelError::Invalid)).map_err(|e| match e {
+        crate::KernelError::Invalid(m) => m,
+        other => other.to_string(),
+    })
+}
+
+fn translate(text: &str) -> std::result::Result<(String, Vec<String>), String> {
     if text.len() > MAX_IGES_BYTES {
         return Err("IGES file too large".into());
     }
     let f = parse(text)?;
     let tol = (f.resolution * f.unit).clamp(1e-6, 0.05).max(1e-5);
-    let mut t = Tr { f: &f, o: Out { lines: Vec::new() }, warnings: Vec::new(), tol, surf_cache: HashMap::new() };
+    let mut t =
+        Tr { f: &f, o: Out { lines: Vec::new() }, warnings: Vec::new(), tol, surf_cache: HashMap::new(), curve_work: 0, composites: Vec::new() };
     // Header: units and context.
     let mm = t.o.add("(LENGTH_UNIT()NAMED_UNIT(*)SI_UNIT(.MILLI.,.METRE.))");
     let rad = t.o.add("(NAMED_UNIT(*)PLANE_ANGLE_UNIT()SI_UNIT($,.RADIAN.))");

crates/kernel/src/iges_in/tests.rs

@@ -157,3 +157,56 @@ fn a_drill_point_round_trips_through_iges() {
     assert!(rel(measure(&drilled).unwrap().volume, v) < 1e-3);
     round_trip(&drilled);
 }
+
+/// A composite curve (102) names its children by pointer, and nothing stops a tree whose every
+/// node names the same child twice. Twenty levels under the depth cap is a million leaf visits
+/// and thirty is 2^30; only counting the work sees that, because the depth never grows past the
+/// cap and a node that yields no segment trips no bound on the output.
+#[test]
+fn composite_curve_fan_out_is_bounded() {
+    let quad = |z: f64| {
+        let c = [[0.0, 0.0, z], [10.0, 0.0, z], [10.0, 20.0, z], [0.0, 20.0, z]];
+        let fmt = |p: [f64; 3]| format!("{:?},{:?},{:?}", p[0], p[1], p[2]);
+        format!("1,1,1,1,0,0,1,0,0,0.,0.,1.,1.,0.,0.,1.,1.,1.,1.,1.,1.,{},{},{},{},0.,1.,0.,1.", fmt(c[0]), fmt(c[1]), fmt(c[3]), fmt(c[2]))
+    };
+    let mut ents: Vec<(i64, i64, String)> = vec![(128, 0, quad(0.0)), (110, 0, "0.,0.,0.,10.,0.,0.".into())];
+    let (surface, mut child) = (1, 3);
+    // Twenty composites, each naming the one below it twice.
+    for _ in 0..20 {
+        ents.push((102, 0, format!("2,{child},{child}")));
+        child = 2 * ents.len() - 1;
+    }
+    ents.push((142, 0, format!("0,{surface},0,{child},2")));
+    let cos = 2 * ents.len() - 1;
+    ents.push((144, 0, format!("{surface},1,0,{cos}")));
+
+    let t = std::time::Instant::now();
+    let reported = match iges_to_step(&iges_file(&ents)) {
+        Ok((_, warnings)) => warnings.join(" "),
+        Err(error) => error,
+    };
+    assert!(reported.contains("too complex") || reported.contains("too many segments"), "{reported}");
+    assert!(t.elapsed() < std::time::Duration::from_secs(10), "took {:?}", t.elapsed());
+}
+
+/// Nothing stops directory entries from all naming the same parameter range, and each one
+/// materialises it as its own string and token list. Ten thousand entries over a million
+/// shared lines retained about a gigabyte each.
+#[test]
+fn directory_entries_cannot_share_their_way_past_the_file_size() {
+    let mut out = vec![format!("{:<72}S{:>7}", "test", 1), format!("{:<72}G{:>7}", "1H,,1H;,,,,,,,,,,,1.,2,2HMM,,,,1.E-6;", 1)];
+    // 1,000 entries, each claiming the same 2,000 parameter lines.
+    for k in 0..1_000 {
+        out.push(format!("{:>8}{:>8}{:>8}{:>8}{:>8}{:>8}{:>8}{:>8}{:>8}D{:>7}", 110, 1, 0, 0, 0, 0, 0, 0, "00000000", 2 * k + 1));
+        out.push(format!("{:>8}{:>8}{:>8}{:>8}{:>8}{:>8}{:>8}{:>8}{:>8}D{:>7}", 110, 0, 0, 2_000, 0, 0, 0, "", 0, 2 * k + 2));
+    }
+    for i in 0..2_000 {
+        out.push(format!("{:<64}{:>8}P{:>7}", "0.,0.,0.,1.,0.,0.;", 1, i + 1));
+    }
+    out.push(format!("{:<72}T{:>7}", "", 1));
+
+    let t = std::time::Instant::now();
+    let error = iges_to_step(&out.join("\n")).unwrap_err();
+    assert!(error.contains("parameter data is too large"), "{error}");
+    assert!(t.elapsed() < std::time::Duration::from_secs(10), "took {:?}", t.elapsed());
+}

crates/kernel/src/step_in/brep.rs

@@ -16,6 +16,10 @@ use crate::guard;
 const MAX_FACES: usize = 500_000;
 /// Most edges in one loop.
 const MAX_LOOP: usize = 100_000;
+/// Most boundary loops on one face. Each bound clones its edge handles into a new wire and the
+/// face extent then samples nine points per edge, so one face listing the same 100,000-edge
+/// loop a million times would hold 1e11 handles.
+const MAX_BOUNDS: usize = 10_000;
 
 /// A body read from a brep entity.
 pub(crate) struct BrepOut {
@@ -196,9 +200,18 @@ impl Builder {
         if !matches!(e.name(), "ADVANCED_FACE" | "FACE_SURFACE" | "FACE") {
             return Err(format!("unsupported face {}", e.name()));
         }
+        let bounds = refs(p.get(1));
+        if bounds.len() > MAX_BOUNDS {
+            return Err(format!("a face with more than {MAX_BOUNDS} boundary loops"));
+        }
         let mut wires = Vec::new();
-        for b in refs(p.get(1)) {
+        let mut edges = 0usize;
+        for b in bounds {
             if let Some(w) = self.bound(cx, b)? {
+                edges = edges.saturating_add(w.len());
+                if edges > MAX_LOOP {
+                    return Err(format!("a face with more than {MAX_LOOP} boundary edges"));
+                }
                 wires.push(w);
             }
         }
@@ -263,7 +276,13 @@ impl Builder {
                 .into_iter()
                 .map(|w| {
                     let edges: Vec<mt::Edge> = w.edge_iter().cloned().collect();
-                    let passes = |e: &mt::Edge| edges.iter().filter(|x| x.front().id() == e.front().id()).count();
+                    // Counted once: a loop may hold 100,000 edges, and asking per edge how
+                    // often its front vertex appears would scan them all again each time.
+                    let mut fronts: HashMap<_, usize> = HashMap::new();
+                    for e in &edges {
+                        *fronts.entry(e.front().id()).or_default() += 1;
+                    }
+                    let passes = |e: &mt::Edge| fronts.get(&e.front().id()).copied().unwrap_or(0);
                     match edges.first().map(passes) {
                         Some(n) if n > 1 => {
                             let start = edges.iter().position(|e| passes(e) == 1).unwrap_or(0);

crates/kernel/src/step_in/geom.rs

@@ -146,7 +146,11 @@ pub(crate) fn axis1(cx: &Ctx, id: u64) -> R<(mt::Point3, mt::Vector3)> {
     Ok((o, d))
 }
 
-fn knot_vec(mults: Option<&Param>, knots: Option<&Param>) -> R<mt::KnotVec> {
+/// `expected` is the knot count the control points and degree imply. Multiplicities are
+/// expanded into one value each, so the sum has to be checked before the expansion: 20 million
+/// knots of multiplicity 64 would build a 10 GB vector that the length check then rejects, and
+/// an allocation failure aborts the process where an error does not.
+fn knot_vec(mults: Option<&Param>, knots: Option<&Param>, expected: usize) -> R<mt::KnotVec> {
     let m: Vec<usize> = mults
         .and_then(Param::as_list)
         .ok_or("B-spline without knot multiplicities")?
@@ -157,6 +161,9 @@ fn knot_vec(mults: Option<&Param>, knots: Option<&Param>) -> R<mt::KnotVec> {
     if m.len() != k.len() || k.is_empty() || k.windows(2).any(|w| w.first() > w.get(1)) {
         return Err("inconsistent knot vector".into());
     }
+    if m.len() > expected || m.iter().try_fold(0usize, |a, v| a.checked_add(*v)) != Some(expected) {
+        return Err("knot multiplicities do not match the control points".into());
+    }
     mt::KnotVec::from_single_multi(k, m).map_err(|e| format!("knot vector: {e}"))
 }
 
@@ -238,7 +245,7 @@ fn bspline_curve(cx: &Ctx, e: &Entity) -> R<CurveGeo> {
     }
     let ctrl: Vec<mt::Point3> = ctrl_ids.iter().map(|c| point(cx, c.as_ref_id().ok_or("bad control point")?)).collect::<R<_>>()?;
     let kv = match knots {
-        Some((m, k)) => knot_vec(m, k)?,
+        Some((m, k)) => knot_vec(m, k, ctrl.len() + degree + 1)?,
         None => generated_knots(kind, ctrl.len(), degree)?,
     };
     if kv.len() != ctrl.len() + degree + 1 {
@@ -737,7 +744,7 @@ pub(crate) fn bspline_surface(cx: &Ctx, e: &Entity) -> R<mt::Surface> {
         return Err("ragged B-spline control net".into());
     }
     let (ku, kv) = match knots {
-        Some([mu, mv, ku, kv]) => (knot_vec(mu, ku)?, knot_vec(mv, kv)?),
+        Some([mu, mv, ku, kv]) => (knot_vec(mu, ku, nu + du + 1)?, knot_vec(mv, kv, nv + dv + 1)?),
         None => (generated_knots(kind, nu, du)?, generated_knots(kind, nv, dv)?),
     };
     if ku.len() != nu + du + 1 || kv.len() != nv + dv + 1 {

crates/kernel/src/step_in/tests.rs

@@ -622,3 +622,57 @@ fn glued_halves_export_as_whole_faces() {
     // Box sides and caps (6) and two holes.
     assert_eq!(imp.bodies[0].file_faces, 8);
 }
+
+/// Knot multiplicities are expanded into one value each before the knot count is compared with
+/// the control points, so the sum has to be checked first: 1,000,000 knots of multiplicity 64
+/// against two control points asked for a 500 MB vector (20,000,000 for 10 GB) only to reject
+/// it afterwards, and an allocation failure aborts the process where an error does not.
+#[test]
+fn knot_multiplicities_are_checked_before_they_are_expanded() {
+    let curve = |n: usize, mult: usize| {
+        let knots: Vec<String> = (0..n).map(|i| format!("{i}.")).collect();
+        let mults: Vec<String> = std::iter::repeat_n(mult.to_string(), n).collect();
+        format!(
+            "{HEAD}#1=CARTESIAN_POINT('',(0.,0.,0.));\n#2=CARTESIAN_POINT('',(1.,0.,0.));\n\
+             #3=B_SPLINE_CURVE_WITH_KNOTS('',1,(#1,#2),.UNSPECIFIED.,.F.,.F.,({}),({}),.UNSPECIFIED.);\n{TAIL}",
+            mults.join(","),
+            knots.join(",")
+        )
+    };
+    let read = |text: &str| {
+        let ex = p21::parse(text).unwrap();
+        let cx = Ctx { ex: &ex, len: 1.0, ang: 1.0, tol: 1e-6 };
+        geom::curve(&cx, 3, 0)
+    };
+    let t = std::time::Instant::now();
+    let error = read(&curve(1_000_000, 64)).unwrap_err();
+    assert!(error.contains("knot multiplicities do not match"), "{error}");
+    assert!(t.elapsed() < std::time::Duration::from_secs(5), "took {:?}", t.elapsed());
+    // A degree-1 curve with two control points needs exactly four knot values.
+    assert!(read(&curve(2, 2)).is_ok());
+    assert!(read(&curve(4, 1)).is_ok());
+    assert!(read(&curve(2, 3)).is_err());
+}
+
+/// One `ADVANCED_FACE` may list any number of bounds, and each one clones its edge handles into
+/// a new wire that the face extent then samples. A face naming the same loop a million times
+/// held 1e11 handles.
+#[test]
+fn faces_with_too_many_bounds_are_refused() {
+    let good = step_export(&[&box_solid(Vec3::ZERO, Vec3::new(5.0, 5.0, 5.0)).unwrap()], "t").unwrap();
+    // One extra face on the cube's shell, listing its outer bound 100,000 times.
+    let bounds = std::iter::repeat_n("#900001", 100_000).collect::<Vec<_>>().join(",");
+    let mut extra = format!("#900000=ADVANCED_FACE('',({bounds}),#900010,.T.);\n");
+    extra += "#900001=FACE_OUTER_BOUND('',#900002,.T.);\n#900002=EDGE_LOOP('',(#900003));\n";
+    extra += "#900003=ORIENTED_EDGE('',*,*,#900004,.T.);\n#900004=EDGE_CURVE('',#900005,#900005,#900007,.T.);\n";
+    extra += "#900005=VERTEX_POINT('',#900006);\n#900006=CARTESIAN_POINT('',(0.,0.,0.));\n";
+    extra += "#900007=LINE('',#900006,#900008);\n#900008=VECTOR('',#900009,1.);\n#900009=DIRECTION('',(1.,0.,0.));\n";
+    extra += "#900010=PLANE('',#900011);\n#900011=AXIS2_PLACEMENT_3D('',#900006,#900012,#900009);\n#900012=DIRECTION('',(0.,0.,1.));\n";
+    let text =
+        good.replacen("CLOSED_SHELL('',(", "CLOSED_SHELL('',(#900000,", 1).replacen("ENDSEC;\nEND-ISO", &format!("{extra}ENDSEC;\nEND-ISO"), 1);
+
+    let start = std::time::Instant::now();
+    let imp = step_import(&text).unwrap();
+    assert!(imp.warnings.iter().any(|w| w.contains("boundary loops")), "{:?}", imp.warnings);
+    assert!(start.elapsed() < std::time::Duration::from_secs(10), "took {:?}", start.elapsed());
+}