fix: bound four unbounded expansions in STEP and IGES import

Each of these asks the allocator for tens of gigabytes, or runs for hours, from
a file of a few kilobytes to a few megabytes. An allocation failure is an abort,
so no panic guard can report it and the user's unsaved work is gone.

- IGES composite curves (102) name their children by pointer, and a tree whose
  every node names the same child twice fans out to 2^30 leaf visits under the
  depth cap. Count the entities visited and the segments produced against one
  budget, since a node that yields no segment trips no bound on the output, and
  refuse a composite curve that contains itself.
- IGES directory entries could all name the same parameter range, each
  materialising it as its own string and token list. Keep the total text
  proportional to the parameter section the file carries.
- STEP knot multiplicities were expanded into one value each before the knot
  count was compared with the control points: 20 million knots of multiplicity
  64 built a 10 GB vector only to be rejected. Check the sum first.
- One STEP face could list any number of boundary loops, each cloning its edge
  handles into a new wire that the face extent then samples. Cap the loops and
  the edges they hold; 100,000 bounds went from 2.1 GB to an immediate error.

Also guard the IGES translation, which was the one importer stage outside every
panic guard, keep the vertex-sewing bucket key clear of the overflow its
neighbour lookup caused for coordinates near f64's range, and count how often a
loop passes each vertex once instead of once per edge.

Fixes #146

Co-Authored-By: Claude Opus 5 <[email protected]>
(cherry picked from commit c32792b71c76d74348ad5002d8d2da870937e58e)
This commit is contained in:
Matt committed 2026-10-11 08:22:07 -07:00
1 parent a094fa46b5
commit b5fb86f245
5 files changed
+199 -12

No files matched your search

+61 -7
View File
@@ -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.))");
+53
View File
@@ -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());
}
+21 -2
View File
@@ -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);
+10 -3
View File
@@ -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 {
+54
View File
@@ -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());
}