google / google/filament

gltfio: heap OOB read + std::vector OOB on malformed KHR_materials_variants mapping (createMaterialVariants)

Open Beginner friendly
#10,192 0 comments 0 reactions 0 assignees View on GitHub
gltf security
Dominant language
C++
Stars
20.5k
Forks
2.3k
Avg merge
2d 14h
Merged PRs (30d)
83

Description

A malformed `KHR_materials_variants` mapping in an untrusted glTF causes a heap out-of-bounds access. `FAssetLoader::createMaterialVariants` uses the per-primitive mapping's `variant` value to index the variant array without clamping to `variants_count`.

**Affected code** — `libs/gltfio/src/AssetLoader.cpp` (`createMaterialVariants`, ~L958–969):
```cpp
const size_t variantIndex = srcPrim.mappings[i].variant; // attacker-controlled, unclamped
... mVariants[variantIndex] ... // OOB read if variantIndex >= variants_count
... .mappings.push_back(...) // OOB std::vector read/write
```

**Repro:** top-level `KHR_materials_variants` with `variants:[{}]` (count = 1) + a primitive mapping with `"variant":1000`. With an ASan gltfio build: `AddressSanitizer: heap-buffer-overflow READ of size 8 ... 40 bytes after 56-byte region` (`std::vector::push_back`).

**Impact:** OOB read of the variant array plus an OOB `std::vector` operation on `push_back` (potential OOB write). Distinct from the `JOINTS_0` issue (different file, different input, OOB write vs read).

**Suggested fix:** guard `variantIndex` against `mVariants.size()` before indexing (skip the malformed mapping, matching the existing "index out of range → continue" handling for TEXCOORD/JOINTS set indices). I previously opened PR #10174 with this fix; it was auto-closed (maintainer-only directory), so filing here as requested.

Contributor guide

Open the contributing guide

Research direction

Start in libs/gltfio/src/AssetLoader.cpp at FAssetLoader::createMaterialVariants, around the reported lines, and inspect how mapping variant values are handled alongside the existing out-of-range checks. Build or run the gltfio ASan reproduction using variants:[{}] and variant:1000. Done means malformed mappings are skipped without out-of-bounds access or vector operations.

Written by the indexing model from the issue text.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.