draco: glTF decoder does not enforce accessor data within its bufferView (cross-view reads in CopyDataAs*)

Open
  • Is something wrong?
#1,260 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 5 days

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
55/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
cpp
Domain
computer-graphics

Research direction

Read tiny_gltf_utils.h and compare the CopyDataAs* byte-range checks with CopyDataFromBufferView in gltf_decoder.cc around lines 392–410. Trace the listed attribute, animation, and skin call paths, then check how their tests construct accessors and bufferViews. Done means accessors whose data exceeds bufferView.byteLength are rejected across these paths, with valid inputs still decoding correctly.

Written by the indexing model from the issue text.

Description

Root cause: the CopyDataAs* variants (CopyDataAsUint32, the CopyDataAs overloads, and CopyDataAsFloatImpl in tiny_gltf_utils.h) check byte_stride * count against the BUFFER end, never against bufferView.byteLength. So an accessor whose declared view is shorter than its count silently consumes the next view's bytes in the same buffer. CopyDataFromBufferView (gltf_decoder.cc:392, byte-range check at :410) shows the correct pattern, but it is used only by structural-metadata property tables — not by any attribute, animation, or skin path.

Value proofs (local ASan build; all files decode SUCCESSFULLY where a view-respecting decoder must reject them; zero sanitizer reports — this is a correctness/validation gap, NOT memory unsafety, and is not presented as heap OOB):

  • TEXCOORD_0: view 8 bytes (one VEC2 float), count 3 (needs 24 bytes) — decoded UVs are identical to an honest 24-byte control file (neighbor view's UVs stolen).
  • Animation: times view 4 bytes (one float), count 2 — decoded times [0, 10] (10 stolen from the neighbor view); translations view 12 bytes (one VEC3), count 2 — second VEC3 stolen from the neighbor view.
  • Skin inverseBindMatrices: view 64 bytes (one MAT4), count 2 (needs 128 bytes) — second matrix is exactly the neighbor view's floats 100..115.
  • The same shape was previously shown for POSITION and indices.

Impact: malformed glTF silently produces wrong geometry, animation, and skinning instead of being rejected. Low severity (correctness/validation).

Suggested fix: enforce accessor.byteOffset + byte_stride * count <= bufferView.byteLength in CopyDataAsUint32, all CopyDataAs overloads, and CopyDataAsFloatImpl — one fix family closes all consumers together.

Note on related work: distinct from #1247 (index upper-bound validation in draco's own decoder, not byteLength enforcement) and from the tinygltf KHR parsing issues (#1257/#1258/#1259, different parser and path).

Reproducer: the multi-view glTF triggers are available on request.

Dominant language
C++
Stars
7.5k
Forks
1.1k
Avg merge
3d 11h
Merged PRs (30d)
9

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from google/draco

All issues in google/draco

Similar issues

More C++ issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.