From 966e27969107278054b912bd35f31274d944ec1c Mon Sep 17 00:00:00 2001 From: Barzabel Date: Thu, 22 Oct 2026 20:10:51 +0300 Subject: [PATCH 1/2] model: cap glTF buffer-less accessors in aggregate, check element offsets (#2139) The per-accessor count checks from #1235 already reject the audit's trigger (a MAT4 accessor with count 10^11); a regression test now pins it. What was left: accessors without a bufferView were capped at 16M components each, but nothing backs them, so one capped accessor referenced from many primitives allocated 64 MB per reference. The cap is now one budget for the whole file. Per-element offsets use checked arithmetic as well. --- crates/model/src/gltf.rs | 35 +++++++++++++++++++++++------------ crates/model/src/tests.rs | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 12 deletions(-) diff --git a/crates/model/src/gltf.rs b/crates/model/src/gltf.rs index e17d014f6..e70dc4fc2 100644 --- a/crates/model/src/gltf.rs +++ b/crates/model/src/gltf.rs @@ -11,7 +11,7 @@ //! `KHR_lights_punctual` lights (directional, point, spot). Sparse accessors and morph targets //! are ignored. -use std::sync::Arc; +use std::{cell::Cell, sync::Arc}; use serde_json::Value; @@ -112,6 +112,9 @@ fn load_uri(uri: &str, resolve: &dyn Fn(&str) -> Option>) -> Result { j: &'a Value, buffers: Vec>, + /// Zero components that buffer-less accessors may still allocate, across the whole file: + /// nothing backs them, so many references to one capped accessor must not multiply (#2139). + zero_left: Cell, } fn arr<'v>(v: &'v Value, k: &str) -> &'v [Value] { @@ -126,8 +129,9 @@ fn indices(v: &Value, k: &str) -> Vec { arr(v, k).iter().filter_map(Value::as_u64).filter_map(|c| usize::try_from(c).ok()).collect() } -/// The most components an accessor without a buffer view (all zeros) may have: nothing in the -/// file backs its count, so it is capped instead of checked against a buffer (#1235). +/// The most components accessors without a buffer view (all zeros) may have, together: nothing +/// in the file backs their counts, so they are capped instead of checked against a buffer +/// (#1235, #2139). const MAX_ZERO_COMPONENTS: usize = 1 << 24; /// Where an accessor's elements lie in its buffer: `count` elements of `elem` bytes, `stride` @@ -150,6 +154,13 @@ fn nums(v: &Value, k: &str, d: [f64; N]) -> [f64; N] { out } +/// Bytes of component `c` of element `i`, with checked offsets (the span was validated, but +/// nothing derived from the file is indexed unchecked). +fn component(buf: &[u8], base: usize, stride: usize, i: usize, c: usize, csize: usize) -> Result<&[u8]> { + let o = i.checked_mul(stride).and_then(|o| o.checked_add(base)).and_then(|o| o.checked_add(c.checked_mul(csize)?)); + o.and_then(|o| buf.get(o..o.checked_add(csize)?)).ok_or_else(|| perr("accessor reads past its buffer")) +} + impl Doc<'_> { /// The accessor's element count and total components (`count * n`), checked; and its span in /// the buffer, validated before anything is allocated from the count. `None` for an @@ -161,16 +172,18 @@ impl Doc<'_> { }; let total = count.checked_mul(n).ok_or_else(|| perr("accessor count is too large"))?; let Some(bv) = idx(acc, "bufferView") else { - if total > MAX_ZERO_COMPONENTS { - return Err(perr(format!("accessor without a bufferView has {total} components (at most {MAX_ZERO_COMPONENTS})"))); - } + let left = self.zero_left.get(); + let Some(rest) = left.checked_sub(total) else { + return Err(perr(format!("accessors without a bufferView have more than {MAX_ZERO_COMPONENTS} components ({total} requested, {left} left)"))); + }; + self.zero_left.set(rest); return Ok((count, total, None)); }; let view = arr(self.j, "bufferViews").get(bv).ok_or_else(|| perr(format!("no bufferView {bv}")))?; let buf = self.buffers.get(idx(view, "buffer").unwrap_or(0)).ok_or_else(|| perr("bufferView buffer out of range"))?; let past = || perr("accessor reads past its buffer"); let base = idx(view, "byteOffset").unwrap_or(0).checked_add(idx(acc, "byteOffset").unwrap_or(0)).ok_or_else(past)?; - let elem = csize * n; + let elem = csize.checked_mul(n).ok_or_else(past)?; let stride = idx(view, "byteStride").filter(|s| *s > 0).unwrap_or(elem); if stride < elem { return Err(perr(format!("bufferView byteStride {stride} is smaller than its {elem}-byte elements"))); @@ -214,8 +227,7 @@ impl Doc<'_> { // Matrices of 1/2-byte components pad columns to 4 bytes; ignored (rare). for i in 0..count { for c in 0..n { - let o = base + i * stride + c * csize; - let s = buf.get(o..o + csize).ok_or_else(|| perr("accessor reads past its buffer"))?; + let s = component(buf, base, stride, i, c, csize)?; out[i * n + c] = match ct { 5126 => f32::from_le_bytes([s[0], s[1], s[2], s[3]]), 5125 => u32::from_le_bytes([s[0], s[1], s[2], s[3]]) as f32, @@ -264,8 +276,7 @@ impl Doc<'_> { let mut out = Vec::with_capacity(total); for i in 0..count { for c in 0..n { - let o = base + i * stride + c * csize; - let s = buf.get(o..o + csize).ok_or_else(|| perr("accessor reads past its buffer"))?; + let s = component(buf, base, stride, i, c, csize)?; out.push(match csize { 1 => s[0] as u32, 2 => u16::from_le_bytes([s[0], s[1]]) as u32, @@ -300,7 +311,7 @@ pub fn parse(bytes: &[u8], resolve: &dyn Fn(&str) -> Option>) -> Result< }; buffers.push(data); } - let d = Doc { j: &j, buffers }; + let d = Doc { j: &j, buffers, zero_left: Cell::new(MAX_ZERO_COMPONENTS) }; // Images → textures (decoded once per image; samplers set the wrap modes). let mut images: Vec>> = vec![]; diff --git a/crates/model/src/tests.rs b/crates/model/src/tests.rs index 7c98c1ae7..7a300e71e 100644 --- a/crates/model/src/tests.rs +++ b/crates/model/src/tests.rs @@ -564,3 +564,35 @@ fn gltf_strided_and_buffer_less_accessors_still_import() { let doc = gltf_doc(r#"{"bufferView": 0, "componentType": 5126, "type": "VEC3", "count": 3}"#, r#"{"buffer": 0, "byteStride": 24}"#, 59); assert!(crate::gltf::parse(doc.as_bytes(), &|_| None).is_err()); } + +#[test] +fn gltf_issue_2139_huge_mat4_count_is_an_error() { + // The audit's trigger (#2139): a MAT4 accessor with a count of 10^11, with and without a + // buffer view, must fail cleanly instead of sizing an allocation from the count. + let view = r#"{"buffer": 0, "byteLength": 64}"#; + for acc in [ + r#"{"bufferView": 0, "componentType": 5126, "type": "MAT4", "count": 100000000000}"#, + r#"{"componentType": 5126, "type": "MAT4", "count": 100000000000}"#, + ] { + let doc = gltf_doc(r#"{"bufferView": 0, "componentType": 5126, "type": "VEC3", "count": 1}"#, view, 64) + .replace(r#""count": 1}]"#, &format!(r#""count": 1}}, {acc}]"#)) + .replace(r#""nodes": [{"mesh": 0}]"#, r#""nodes": [{"mesh": 0, "skin": 0}], "skins": [{"joints": [0], "inverseBindMatrices": 1}]"#); + assert!(doc.contains("inverseBindMatrices"), "{doc}"); + assert!(crate::gltf::parse(doc.as_bytes(), &|_| None).is_err(), "{acc}"); + } +} + +#[test] +fn gltf_buffer_less_accessors_share_one_zero_budget() { + // Each buffer-less accessor is capped, but nothing backs any of them, so referencing a capped + // one from many primitives must not multiply the allocation (#2139). + let acc = r#"{"componentType": 5126, "type": "VEC3", "count": 5000000}"#; + let prims = [r#"{"attributes": {"POSITION": 0}}"#; 4].join(", "); + let doc = gltf_doc(acc, r#"{"buffer": 0}"#, 3).replace(r#"[{"attributes": {"POSITION": 0}}]"#, &format!("[{prims}]")); + assert_eq!(doc.matches(r#""POSITION": 0"#).count(), 4); + let e = crate::gltf::parse(doc.as_bytes(), &|_| None).err().unwrap_or_else(|| panic!("imported")); + assert!(e.to_string().contains("without a bufferView"), "{e}"); + // One such accessor read once still imports. + let doc = gltf_doc(acc, r#"{"buffer": 0}"#, 3); + assert!(crate::gltf::parse(doc.as_bytes(), &|_| None).is_ok()); +} From 9abdd74c1bb9d3b27a8cee4397ca99a58d8c0e4a Mon Sep 17 00:00:00 2001 From: Barzabel Date: Mon, 26 Oct 2026 20:38:03 +0300 Subject: [PATCH 2/2] model: one per-file glTF read budget for every accessor read (#2139) Review of #2158: a buffer-backed accessor is checked against its buffer, but each primitive that references it reads it again and keeps its own copy, so a 3 MB file listing 50,000 references to one 100,000-vertex accessor asked for about 80 GB. Every accessor read, with or without a bufferView, is now charged to one budget for the whole file, as is what a primitive keeps besides its reads: generated indices when `indices` is missing, the triangle list (3(k-2) for a strip or fan of k), flat-normal vertices and computed tangents. Going over it is the usual malformed-file error. The cap is a fixed 2^28 components (about 1 GiB as f32/u32), not a multiple of the buffer size: real assets share one vertex buffer across many primitives, and a 10M-triangle scan reads about 90M components. Buffer-less accessors keep their tighter 16M sub-budget. --- crates/model/src/gltf.rs | 52 +++++++++++++++++++++++++++++++++++++-- crates/model/src/tests.rs | 49 ++++++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 2 deletions(-) diff --git a/crates/model/src/gltf.rs b/crates/model/src/gltf.rs index e70dc4fc2..0656c9219 100644 --- a/crates/model/src/gltf.rs +++ b/crates/model/src/gltf.rs @@ -115,6 +115,11 @@ struct Doc<'a> { /// Zero components that buffer-less accessors may still allocate, across the whole file: /// nothing backs them, so many references to one capped accessor must not multiply (#2139). zero_left: Cell, + /// Components every accessor read may still allocate, across the whole file: each primitive + /// that references an accessor keeps its own copy, so references must not multiply memory. + read_left: Cell, + /// The budget `read_left` started from, for the error message. + read_cap: usize, } fn arr<'v>(v: &'v Value, k: &str) -> &'v [Value] { @@ -134,6 +139,13 @@ fn indices(v: &Value, k: &str) -> Vec { /// (#1235, #2139). const MAX_ZERO_COMPONENTS: usize = 1 << 24; +/// The most components all accessor reads in one file may allocate together (about 1 GiB as +/// `f32`/`u32`). Each primitive keeps its own copy of what it reads, so an accessor referenced +/// N times costs N times; this bounds that by a fixed amount rather than by buffer size, because +/// real assets legitimately share one vertex buffer across many primitives. A 10M-triangle scan +/// (5M vertices with position, normal, UV and tangent, 30M indices) reads about 90M. +const MAX_READ_COMPONENTS: usize = 1 << 28; + /// Where an accessor's elements lie in its buffer: `count` elements of `elem` bytes, `stride` /// apart from `base`, all checked to be inside `buf`. struct Span<'b> { @@ -162,6 +174,22 @@ fn component(buf: &[u8], base: usize, stride: usize, i: usize, c: usize, csize: } impl Doc<'_> { + /// Charge `total` components to the file's read budget before allocating them. + fn charge(&self, total: usize) -> Result<()> { + let left = self.read_left.get(); + let Some(rest) = left.checked_sub(total) else { + let cap = self.read_cap; + return Err(perr(format!("accessor reads need more than {cap} components in total ({total} requested, {left} left)"))); + }; + self.read_left.set(rest); + Ok(()) + } + + /// Charge `count` items of `per` components each. + fn charge_each(&self, count: usize, per: usize) -> Result<()> { + self.charge(count.checked_mul(per).ok_or_else(|| perr("primitive is too large"))?) + } + /// The accessor's element count and total components (`count * n`), checked; and its span in /// the buffer, validated before anything is allocated from the count. `None` for an /// accessor without a buffer view, whose components are all zero. @@ -177,6 +205,7 @@ impl Doc<'_> { return Err(perr(format!("accessors without a bufferView have more than {MAX_ZERO_COMPONENTS} components ({total} requested, {left} left)"))); }; self.zero_left.set(rest); + self.charge(total)?; return Ok((count, total, None)); }; let view = arr(self.j, "bufferViews").get(bv).ok_or_else(|| perr(format!("no bufferView {bv}")))?; @@ -194,6 +223,7 @@ impl Doc<'_> { return Err(past()); } } + self.charge(total)?; Ok((count, total, Some(Span { buf, base, stride }))) } @@ -290,6 +320,11 @@ impl Doc<'_> { /// Parse a `.gltf` (JSON) or `.glb` file. pub fn parse(bytes: &[u8], resolve: &dyn Fn(&str) -> Option>) -> Result { + parse_with_budget(bytes, resolve, MAX_READ_COMPONENTS) +} + +/// [`parse`] with a read budget of `read_budget` accessor components for the whole file. +pub(crate) fn parse_with_budget(bytes: &[u8], resolve: &dyn Fn(&str) -> Option>, read_budget: usize) -> Result { let (json, bin) = if bytes.starts_with(b"glTF") { split_glb(bytes)? } else { (bytes, None) }; let j: Value = serde_json::from_slice(json).map_err(|e| perr(format!("glTF JSON: {e}")))?; let ver = j.get("asset").and_then(|a| a.get("version")).and_then(Value::as_str).unwrap_or(""); @@ -311,7 +346,7 @@ pub fn parse(bytes: &[u8], resolve: &dyn Fn(&str) -> Option>) -> Result< }; buffers.push(data); } - let d = Doc { j: &j, buffers, zero_left: Cell::new(MAX_ZERO_COMPONENTS) }; + let d = Doc { j: &j, buffers, zero_left: Cell::new(MAX_ZERO_COMPONENTS), read_left: Cell::new(read_budget), read_cap: read_budget }; // Images → textures (decoded once per image; samplers set the wrap modes). let mut images: Vec>> = vec![]; @@ -578,7 +613,11 @@ fn primitive(d: &Doc, p: &Value, materials: &[Material]) -> Result = match idx(p, "indices") { Some(a) => d.ints(a)?, - None => (0..n as u32).collect(), + None => { + // Generated indices are allocated like a read, once per referencing primitive. + d.charge(n)?; + (0..n as u32).collect() + } }; if raw.iter().any(|&i| i as usize >= n) { return Err(perr("index out of range")); @@ -598,11 +637,19 @@ fn primitive(d: &Doc, p: &Value, materials: &[Material]) -> Result indices.extend(raw.as_chunks::<3>().0.iter().flatten()), } + // The triangle list is kept besides the raw indices (a strip or fan of k indices becomes + // 3(k-2)), so it is charged like a read. + d.charge(indices.len())?; if normals.len() != n { normals.clear(); } let material = idx(p, "material").filter(|&m| m < materials.len()); let skinned = joints.len() == n && weights.len() == n; + if normals.is_empty() { + // Flat normals unshare every vertex: position and normal, plus UVs and joints/weights. + let per = 6 + if uvs.len() == n { 2 } else { 0 } + if skinned { 8 } else { 0 }; + d.charge_each(indices.len(), per)?; + } let mut prim = Primitive { positions, normals, @@ -619,6 +666,7 @@ fn primitive(d: &Doc, p: &Value, materials: &[Material]) -> Result String { + let acc = format!(r#"{{"bufferView": 0, "componentType": 5126, "type": "VEC3", "count": {verts}}}"#); + let prims = vec![r#"{"attributes": {"POSITION": 0}}"#; refs].join(", "); + gltf_doc(&acc, r#"{"buffer": 0}"#, verts * 12).replace(r#"[{"attributes": {"POSITION": 0}}]"#, &format!("[{prims}]")) +} + +#[test] +fn gltf_buffer_backed_reads_share_one_read_budget() { + // A buffer-backed accessor is checked against its buffer, but each primitive that references + // it reads and keeps its own copy, so a small file listing many references must not ask for + // memory set by the reference count (#2139 review). Everything a primitive keeps is charged + // to one per-file budget: 1,000 vertices without indices or normals cost 3,000 position + // components, 1,000 generated indices, a 999-index triangle list and 999 flat-shaded + // vertices of 6 components, 10,993 in all. + let one = gltf_shared_positions(1000, 1); + let m = crate::gltf::parse_with_budget(one.as_bytes(), &|_| None, 20_000).unwrap(); + assert_eq!(m.meshes[0].primitives.len(), 1); + let three = gltf_shared_positions(1000, 3); + assert_eq!(three.matches(r#""POSITION": 0"#).count(), 3); + let e = crate::gltf::parse_with_budget(three.as_bytes(), &|_| None, 20_000).err().unwrap_or_else(|| panic!("imported")); + assert!(e.to_string().contains("more than 20000 components"), "{e}"); + // The real cap leaves room for legitimate sharing: many primitives over one vertex buffer. + let m = crate::gltf::parse(gltf_shared_positions(1000, 50).as_bytes(), &|_| None).unwrap(); + assert_eq!(m.meshes[0].primitives.len(), 50); +} + +#[test] +fn gltf_missing_indices_are_charged_to_the_read_budget() { + // Without `indices` every vertex gets a generated index, allocated like a read: after the + // 3,000 position components, 999 left is one short for the 1,000 indices. + let doc = gltf_shared_positions(1000, 1); + let e = crate::gltf::parse_with_budget(doc.as_bytes(), &|_| None, 3999).err().unwrap_or_else(|| panic!("imported")); + assert!(e.to_string().contains("(1000 requested, 999 left)"), "{e}"); +} + +#[test] +fn gltf_strip_expansion_is_charged_to_the_read_budget() { + // A strip of k indices becomes 3(k-2) triangle indices and as many flat-shaded vertices, so + // the output is charged, not only the 1,009 components read. + let doc = gltf_doc(r#"{"bufferView": 0, "componentType": 5126, "type": "VEC3", "count": 3}"#, r#"{"buffer": 0}"#, 1036) + .replace(r#""count": 3}]"#, r#""count": 3}, {"bufferView": 0, "componentType": 5121, "type": "SCALAR", "count": 1000}]"#) + .replace(r#""attributes": {"POSITION": 0}"#, r#""attributes": {"POSITION": 0}, "indices": 1, "mode": 5"#); + let m = crate::gltf::parse_with_budget(doc.as_bytes(), &|_| None, 30_000).unwrap(); + assert_eq!(m.meshes[0].primitives[0].indices.len(), 2994); + let e = crate::gltf::parse_with_budget(doc.as_bytes(), &|_| None, 5000).err().unwrap_or_else(|| panic!("imported")); + assert!(e.to_string().contains("more than 5000 components"), "{e}"); +}