mirror of
https://github.com/storytold/effectcraft.git
synced 2026-10-22 03:32:39 -05:00
model: validate glTF accessor spans before allocating, with checked arithmetic (#1235)
Accessor counts sized the f32/u32 output before the buffer view was looked at, and offsets, strides and count * components used unchecked arithmetic, so a few hundred bytes of JSON could request a multi-gigabyte allocation (aborting the process) or overflow. - Doc::layout checks count * components, adds the view and accessor offsets, rejects a byteStride smaller than an element, and checks that the last element ends inside the buffer, all with checked arithmetic, before floats() or ints() allocate anything. - Accessors without a buffer view (all zeros) are capped at 2^24 components. - idx() and node index lists use usize::try_from instead of 'as usize'; an image bufferView's end and a GLB chunk's end are computed with checked_add. Fixes #1235 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N9kwMcw2pYQ5fSjKa3ZPnh
This commit is contained in:
1 parent
549e9d8251
commit
3d45046132
2 files changed
+137
-22
No files matched your search
+65
-22
@@ -39,13 +39,15 @@ pub fn split_glb(b: &[u8]) -> Result<(&[u8], Option<&[u8]>)> {
|
||||
while o + 8 <= total {
|
||||
let len = u32_at(o).unwrap_or(0) as usize;
|
||||
let ty = u32_at(o + 4).unwrap_or(0);
|
||||
let data = b.get(o + 8..o + 8 + len).ok_or_else(|| perr("GLB chunk runs past the end of the file"))?;
|
||||
let past_end = || perr("GLB chunk runs past the end of the file");
|
||||
let end = (o + 8).checked_add(len).ok_or_else(past_end)?;
|
||||
let data = b.get(o + 8..end).ok_or_else(past_end)?;
|
||||
match ty {
|
||||
CHUNK_JSON if json.is_none() => json = Some(data),
|
||||
CHUNK_BIN if bin.is_none() => bin = Some(data),
|
||||
_ => {}
|
||||
}
|
||||
o += 8 + len;
|
||||
o = end;
|
||||
}
|
||||
Ok((json.ok_or_else(|| perr("GLB without a JSON chunk"))?, bin))
|
||||
}
|
||||
@@ -115,8 +117,25 @@ struct Doc<'a> {
|
||||
fn arr<'v>(v: &'v Value, k: &str) -> &'v [Value] {
|
||||
v.get(k).and_then(Value::as_array).map(Vec::as_slice).unwrap_or(&[])
|
||||
}
|
||||
/// A non-negative integer field; one too large for `usize` counts as absent (#1235).
|
||||
fn idx(v: &Value, k: &str) -> Option<usize> {
|
||||
v.get(k).and_then(Value::as_u64).map(|x| x as usize)
|
||||
v.get(k).and_then(Value::as_u64).and_then(|x| usize::try_from(x).ok())
|
||||
}
|
||||
/// Node indices in an array field (out-of-range ones are dropped).
|
||||
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).
|
||||
const MAX_ZERO_COMPONENTS: usize = 1 << 24;
|
||||
|
||||
/// 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> {
|
||||
buf: &'b [u8],
|
||||
base: usize,
|
||||
stride: usize,
|
||||
}
|
||||
fn num(v: &Value, k: &str, d: f64) -> f64 {
|
||||
v.get(k).and_then(Value::as_f64).unwrap_or(d)
|
||||
@@ -132,6 +151,39 @@ fn nums<const N: usize>(v: &Value, k: &str, d: [f64; N]) -> [f64; N] {
|
||||
}
|
||||
|
||||
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
|
||||
/// accessor without a buffer view, whose components are all zero.
|
||||
fn layout(&self, acc: &Value, n: usize, csize: usize) -> Result<(usize, usize, Option<Span<'_>>)> {
|
||||
let count = match acc.get("count") {
|
||||
None => 0,
|
||||
Some(c) => c.as_u64().and_then(|c| usize::try_from(c).ok()).ok_or_else(|| perr("accessor count is not a valid size"))?,
|
||||
};
|
||||
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})")));
|
||||
}
|
||||
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 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")));
|
||||
}
|
||||
if count > 0 {
|
||||
let end = (count - 1).checked_mul(stride).and_then(|o| o.checked_add(base)).and_then(|o| o.checked_add(elem)).ok_or_else(past)?;
|
||||
if end > buf.len() {
|
||||
return Err(past());
|
||||
}
|
||||
}
|
||||
Ok((count, total, Some(Span { buf, base, stride })))
|
||||
}
|
||||
|
||||
/// An accessor as `f32` rows of `n` components (normalised integers mapped to 0..1 / -1..1).
|
||||
fn floats(&self, a: usize) -> Result<(Vec<f32>, usize)> {
|
||||
let acc = arr(self.j, "accessors").get(a).ok_or_else(|| perr(format!("no accessor {a}")))?;
|
||||
@@ -145,7 +197,6 @@ impl Doc<'_> {
|
||||
"MAT4" => 16,
|
||||
t => return Err(perr(format!("accessor type `{t}`"))),
|
||||
};
|
||||
let count = idx(acc, "count").unwrap_or(0);
|
||||
let ct = idx(acc, "componentType").unwrap_or(5126);
|
||||
let norm = acc.get("normalized").and_then(Value::as_bool).unwrap_or(false);
|
||||
let csize = match ct {
|
||||
@@ -154,17 +205,13 @@ impl Doc<'_> {
|
||||
5125 | 5126 => 4,
|
||||
_ => return Err(perr(format!("componentType {ct}"))),
|
||||
};
|
||||
let mut out = vec![0.0f32; count * n];
|
||||
let Some(bv) = idx(acc, "bufferView") else {
|
||||
let (count, total, span) = self.layout(acc, n, csize)?;
|
||||
let mut out = vec![0.0f32; total];
|
||||
let Some(Span { buf, base, stride }) = span else {
|
||||
// No buffer view: all zeros (sparse substitution is not supported).
|
||||
return Ok((out, n));
|
||||
};
|
||||
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 base = idx(view, "byteOffset").unwrap_or(0) + idx(acc, "byteOffset").unwrap_or(0);
|
||||
let elem = csize * n;
|
||||
// Matrices of 1/2-byte components pad columns to 4 bytes; ignored (rare).
|
||||
let stride = idx(view, "byteStride").filter(|s| *s > 0).unwrap_or(elem);
|
||||
for i in 0..count {
|
||||
for c in 0..n {
|
||||
let o = base + i * stride + c * csize;
|
||||
@@ -207,18 +254,14 @@ impl Doc<'_> {
|
||||
"VEC2" => 2,
|
||||
_ => 1,
|
||||
};
|
||||
let count = idx(acc, "count").unwrap_or(0);
|
||||
let csize = match ct {
|
||||
5120 | 5121 => 1,
|
||||
5122 | 5123 => 2,
|
||||
_ => 4,
|
||||
};
|
||||
let Some(bv) = idx(acc, "bufferView") else { return Ok(vec![0; count * n]) };
|
||||
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 base = idx(view, "byteOffset").unwrap_or(0) + idx(acc, "byteOffset").unwrap_or(0);
|
||||
let stride = idx(view, "byteStride").filter(|s| *s > 0).unwrap_or(csize * n);
|
||||
let mut out = Vec::with_capacity(count * n);
|
||||
let (count, total, span) = self.layout(acc, n, csize)?;
|
||||
let Some(Span { buf, base, stride }) = span else { return Ok(vec![0; total]) };
|
||||
let mut out = Vec::with_capacity(total);
|
||||
for i in 0..count {
|
||||
for c in 0..n {
|
||||
let o = base + i * stride + c * csize;
|
||||
@@ -268,7 +311,7 @@ pub fn parse(bytes: &[u8], resolve: &dyn Fn(&str) -> Option<Vec<u8>>) -> Result<
|
||||
arr(&j, "bufferViews").get(bv).and_then(|v| {
|
||||
let b = d.buffers.get(idx(v, "buffer").unwrap_or(0))?;
|
||||
let o = idx(v, "byteOffset").unwrap_or(0);
|
||||
b.get(o..o + idx(v, "byteLength").unwrap_or(0)).map(<[u8]>::to_vec)
|
||||
b.get(o..o.checked_add(idx(v, "byteLength").unwrap_or(0))?).map(<[u8]>::to_vec)
|
||||
})
|
||||
} else {
|
||||
None
|
||||
@@ -366,13 +409,13 @@ pub fn parse(bytes: &[u8], resolve: &dyn Fn(&str) -> Option<Vec<u8>>) -> Result<
|
||||
skin: idx(n, "skin"),
|
||||
camera: idx(n, "camera"),
|
||||
light: n.get("extensions").and_then(|e| e.get("KHR_lights_punctual")).and_then(|e| idx(e, "light")),
|
||||
children: arr(n, "children").iter().filter_map(Value::as_u64).map(|c| c as usize).collect(),
|
||||
children: indices(n, "children"),
|
||||
});
|
||||
}
|
||||
|
||||
let scene = idx(&j, "scene").unwrap_or(0);
|
||||
let roots: Vec<usize> = match arr(&j, "scenes").get(scene) {
|
||||
Some(s) => arr(s, "nodes").iter().filter_map(Value::as_u64).map(|c| c as usize).collect(),
|
||||
Some(s) => indices(s, "nodes"),
|
||||
None => {
|
||||
// No scene: every node that is nobody's child.
|
||||
let mut child = vec![false; nodes.len()];
|
||||
@@ -389,7 +432,7 @@ pub fn parse(bytes: &[u8], resolve: &dyn Fn(&str) -> Option<Vec<u8>>) -> Result<
|
||||
|
||||
let mut skins = vec![];
|
||||
for s in arr(&j, "skins") {
|
||||
let joints: Vec<usize> = arr(s, "joints").iter().filter_map(Value::as_u64).map(|c| c as usize).collect();
|
||||
let joints: Vec<usize> = indices(s, "joints");
|
||||
let inverse_bind = match idx(s, "inverseBindMatrices") {
|
||||
Some(a) => {
|
||||
let (f, _) = d.floats(a)?;
|
||||
|
||||
@@ -492,3 +492,75 @@ fn stl_hostile_input_is_an_error() {
|
||||
let m = stl::parse(&binary_stl(b"", &[([1.0, 0.0, 0.0], [[0.0; 3]; 3], 0)])).unwrap();
|
||||
assert_eq!(m.meshes[0].primitives[0].normals[0], [1.0, 0.0, 0.0]);
|
||||
}
|
||||
|
||||
/// A one-triangle glTF whose POSITION is accessor 0, over a buffer of `bytes` zero bytes.
|
||||
fn gltf_doc(accessors: &str, views: &str, bytes: usize) -> String {
|
||||
let b64 = "A".repeat(bytes / 3 * 4) + ["", "AA==", "AAA="][bytes % 3];
|
||||
format!(
|
||||
r#"{{"asset": {{"version": "2.0"}}, "scenes": [{{"nodes": [0]}}], "nodes": [{{"mesh": 0}}],
|
||||
"meshes": [{{"primitives": [{{"attributes": {{"POSITION": 0}}}}]}}],
|
||||
"buffers": [{{"byteLength": {bytes}, "uri": "data:application/octet-stream;base64,{b64}"}}],
|
||||
"bufferViews": [{views}], "accessors": [{accessors}]}}"#
|
||||
)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn gltf_accessor_counts_are_checked_against_the_buffer_before_allocating() {
|
||||
// Untrusted counts, offsets and strides must fail the import, not allocate from the count or
|
||||
// overflow (#1235).
|
||||
let view = r#"{"buffer": 0, "byteLength": 36}"#;
|
||||
let cases = [
|
||||
// A count far beyond the 36-byte buffer.
|
||||
(r#"{"bufferView": 0, "componentType": 5126, "type": "VEC3", "count": 4000000000}"#, view, "past its buffer"),
|
||||
// count * components overflows.
|
||||
(r#"{"bufferView": 0, "componentType": 5126, "type": "MAT4", "count": 9223372036854775807}"#, view, "too large"),
|
||||
// Offsets that overflow when added.
|
||||
(
|
||||
r#"{"bufferView": 0, "componentType": 5126, "type": "VEC3", "count": 3, "byteOffset": 16}"#,
|
||||
r#"{"buffer": 0, "byteOffset": 18446744073709551610}"#,
|
||||
"past its buffer",
|
||||
),
|
||||
// A stride that overflows with the count.
|
||||
(r#"{"bufferView": 0, "componentType": 5126, "type": "VEC3", "count": 3000000000}"#, r#"{"buffer": 0, "byteStride": 252}"#, "past its buffer"),
|
||||
// A stride smaller than an element would re-read the same bytes for a huge count.
|
||||
(r#"{"bufferView": 0, "componentType": 5126, "type": "MAT4", "count": 30}"#, r#"{"buffer": 0, "byteStride": 1}"#, "smaller than"),
|
||||
// No buffer view: nothing backs the count, so it is capped.
|
||||
(r#"{"componentType": 5126, "type": "VEC3", "count": 1000000000}"#, view, "without a bufferView"),
|
||||
];
|
||||
let t0 = std::time::Instant::now();
|
||||
for (acc, v, want) in cases {
|
||||
let doc = gltf_doc(acc, v, 36);
|
||||
let e = crate::gltf::parse(doc.as_bytes(), &|_| None).err().unwrap_or_else(|| panic!("{acc} imported"));
|
||||
assert!(e.to_string().contains(want), "{acc}: {e}");
|
||||
}
|
||||
assert!(t0.elapsed() < std::time::Duration::from_secs(10), "{:?}", t0.elapsed());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn gltf_integer_accessors_are_checked_too() {
|
||||
let doc = gltf_doc(r#"{"bufferView": 0, "componentType": 5126, "type": "VEC3", "count": 3}"#, r#"{"buffer": 0, "byteLength": 36}"#, 36)
|
||||
.replace(r#""attributes": {"POSITION": 0}"#, r#""attributes": {"POSITION": 0}, "indices": 1"#);
|
||||
for idx in [
|
||||
r#"{"bufferView": 0, "componentType": 5125, "type": "SCALAR", "count": 4000000000}"#,
|
||||
r#"{"componentType": 5125, "type": "SCALAR", "count": 4000000000}"#,
|
||||
r#"{"bufferView": 0, "componentType": 5125, "type": "VEC4", "count": 9223372036854775807}"#,
|
||||
] {
|
||||
let doc = doc.replace(r#""count": 3}]"#, &format!(r#""count": 3}}, {idx}]"#));
|
||||
assert!(crate::gltf::parse(doc.as_bytes(), &|_| None).is_err(), "{idx}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn gltf_strided_and_buffer_less_accessors_still_import() {
|
||||
// Interleaved positions (stride 24) ending exactly at the buffer's end, and a small all-zero
|
||||
// accessor without a view.
|
||||
let doc = gltf_doc(r#"{"bufferView": 0, "componentType": 5126, "type": "VEC3", "count": 3}"#, r#"{"buffer": 0, "byteStride": 24}"#, 60);
|
||||
let m = crate::gltf::parse(doc.as_bytes(), &|_| None).unwrap();
|
||||
assert_eq!(m.meshes[0].primitives[0].positions.len(), 3);
|
||||
let doc = gltf_doc(r#"{"componentType": 5126, "type": "VEC3", "count": 6}"#, r#"{"buffer": 0}"#, 3);
|
||||
let m = crate::gltf::parse(doc.as_bytes(), &|_| None).unwrap();
|
||||
assert_eq!(m.meshes[0].primitives[0].positions.len(), 6);
|
||||
// One byte short of the third element is an error.
|
||||
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());
|
||||
}
|
||||
Reference in new issue
Block a user