diff --git a/crates/model/src/gltf.rs b/crates/model/src/gltf.rs index 20578d4f..e17d014f 100644 --- a/crates/model/src/gltf.rs +++ b/crates/model/src/gltf.rs @@ -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 { - 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 { + 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(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>)> { + 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, 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>) -> 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>) -> 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 = 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>) -> Result< let mut skins = vec![]; for s in arr(&j, "skins") { - let joints: Vec = arr(s, "joints").iter().filter_map(Value::as_u64).map(|c| c as usize).collect(); + let joints: Vec = indices(s, "joints"); let inverse_bind = match idx(s, "inverseBindMatrices") { Some(a) => { let (f, _) = d.floats(a)?; diff --git a/crates/model/src/tests.rs b/crates/model/src/tests.rs index 9206ef83..7c98c1ae 100644 --- a/crates/model/src/tests.rs +++ b/crates/model/src/tests.rs @@ -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()); +}