aleleba/effectcraft
1
0
Fork 0
mirror of https://github.com/storytold/effectcraft.git synced 2026-10-29 20:47:18 -06:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #2158 from Barzabel/fix/2139-gltf-accessor-bounds

Browse Source 
model: cap glTF buffer-less accessors per file, check element offsets (#2139)
This commit is contained in:
BFlat
2026-10-30 01:16:03 +07:00
committed by GitHub
parent bee53b40da 9abdd74c1b
commit 742f2a2bdd
2 changed files with 153 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/model/src/gltf.rs
+72 -13
View File
@@ -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;
@@ -114,6 +114,14 @@ fn load_uri(uri: &str, resolve: &dyn Fn(&str) -> Option<Vec<u8>>) -> Result<Vec<
struct Doc<'a> {
j: &'a Value,
buffers: Vec<Vec<u8>>,
/// 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<usize>,
/// 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<usize>,
/// The budget `read_left` started from, for the error message.
read_cap: usize,
}
fn arr<'v>(v: &'v Value, k: &str) -> &'v [Value] {
@@ -128,10 +136,18 @@ fn indices(v: &Value, k: &str) -> Vec<usize> {
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;
/// 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> {
@@ -152,7 +168,30 @@ fn nums<const N: usize>(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<'_> {
/// 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.
@@ -163,16 +202,19 @@ 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);
self.charge(total)?;
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")));
@@ -183,6 +225,7 @@ impl Doc<'_> {
return Err(past());
}
}
self.charge(total)?;
Ok((count, total, Some(Span { buf, base, stride })))
}
@@ -216,8 +259,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,
@@ -266,8 +308,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,
@@ -281,6 +322,11 @@ impl Doc<'_> {
/// Parse a `.gltf` (JSON) or `.glb` file.
pub fn parse(bytes: &[u8], resolve: &dyn Fn(&str) -> Option<Vec<u8>>) -> Result<Model> {
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<Vec<u8>>, read_budget: usize) -> Result<Model> {
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("");
@@ -302,7 +348,7 @@ pub fn parse(bytes: &[u8], resolve: &dyn Fn(&str) -> Option<Vec<u8>>) -> Result<
};
buffers.push(data);
}
let d = Doc { j: &j, buffers };
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<Option<Arc<Texture>>> = vec![];
@@ -569,7 +615,11 @@ fn primitive(d: &Doc, p: &Value, materials: &[Material]) -> Result<Option<Primit
};
let raw: Vec<u32> = 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"));
@@ -589,11 +639,19 @@ fn primitive(d: &Doc, p: &Value, materials: &[Material]) -> Result<Option<Primit
}
_ => 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,
@@ -610,6 +668,7 @@ fn primitive(d: &Doc, p: &Value, materials: &[Material]) -> Result<Option<Primit
}
let needs_tangents = material.is_some_and(|m| materials[m].normal_tex.is_some());
if needs_tangents && prim.tangents.is_empty() {
d.charge_each(prim.positions.len(), 4)?;
prim.compute_tangents();
}
Ok(Some(prim))
crates/model/src/tests.rs
+81
View File
@@ -564,3 +564,84 @@ 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());
}
/// `refs` primitives sharing one buffer-backed `verts`-vertex POSITION accessor, without indices.
fn gltf_shared_positions(verts: usize, refs: usize) -> 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}");
}