draco: OOB read/SEGV in glTF animation channel and sampler indices (unchecked indices in AddChannelToAnimation/AddSamplerToAnimation)

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

Maintainers usually reply within 1 day

@Subhan1794 is already working on this.

Since Oct 23, 2026.

  • #1262 by @Subhan1794 — open

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
20/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Stale
Tech stack
cpp
Domain
computer-graphics, security

Research direction

Read src/draco/io/tiny_gltf_utils.cc, especially AddChannelToAnimation and AddSamplerToAnimation, and compare the existing inverseBindMatrices bounds check in gltf_decoder.cc. The issue reports crashes for out-of-range channel.sampler, sampler.input, and sampler.output indices; verify those cases reject invalid indices before vector access. Done means the invalid animation inputs are rejected safely while the valid animation control still decodes; linked pull request #1262 is already open.

Written by the indexing model from the issue text.

Description

Root cause: the glTF animation path indexes model vectors with attacker-controlled indices and no bounds check. In src/draco/io/tiny_gltf_utils.cc, AddChannelToAnimation reads input_animation.samplers[channel.sampler] directly, and AddSamplerToAnimation reads model.accessors[sampler.input] and model.accessors[sampler.output] directly. The vendored tinygltf parser only checks that these fields are present integers — it never validates their range — so a crafted value flows straight into operator[]. Reachable via the public scene path: DecodeFromFileToScene -> DecodeGltfToScene -> AddAnimationsToScene -> AddChannelToAnimation -> AddSamplerToAnimation. Contrast: skin inverseBindMatrices IS guarded (gltf_decoder.cc checks against accessors.size()), and primitive attributes/indices are guarded — the animation path was missed.

Evidence (local ASan+UBSan build, scene decode path, valid animation control decodes clean):

  • Valid control (sampler 0, input 0, output 0): decodes ok.
  • channel.sampler = 5 with only 1 sampler: ASan heap-buffer-overflow READ size 4 (the wild sampler object read then faults downstream — one unchecked chain, one fix covers both).
  • sampler.input = 999: ASan SEGV READ.
  • sampler.output = 999: ASan SEGV READ.
  • channel.sampler = -1 and sampler.input = -1: ASan SEGV READ.
  • No WRITE found in any case.

Impact: crash (denial of service) in any tool decoding glTF scenes with animations through this path. Low severity.

Suggested fix: range checks on channel.sampler, sampler.input, and sampler.output mirroring the existing skin guard, rejecting out-of-range indices before indexing.

Note: distinct from #1247, which covers bufferView/buffer and accessor indices on the primitive path — this is a different function/file/path (animation).

Reproducer: the minimal glTF triggers are available on request.

Dominant language
C++
Stars
7.5k
Forks
1.1k
Avg merge
1d 59m
Merged PRs (30d)
4

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.