gltfio: heap out-of-bounds write in `TangentsJob` via unvalidated `MAT4`-typed morph normal/tangent accessor (type confusion)
- Dominant language
- C++
- Stars
- 20.5k
- Forks
- 2.3k
- Avg merge
- 2d 14h
- Merged PRs (30d)
- 83
Description
### Summary
Loading an untrusted `.glb`/`.gltf` can cause a heap out-of-bounds write in `TangentsJob`, because `gltfio` never validates the element **type** of a morph-target NORMAL/TANGENT accessor. A morph tangent accessor declaring a wide element type (e.g. `MAT4`, 16 components) passes all validation and is later unpacked into a scratch buffer sized for 3 floats per vertex, overflowing it. This is the **default, all-platform loader** path.
(Filing as an issue: I originally opened this as PR #10179 with a fix + regression test, but the `libs/gltfio` automation auto-closes non-maintainer PRs and asks contributors to open an issue first, so here it is.)
### Affected code (branch `main`)
In `FAssetLoader::createPrimitive` (`libs/gltfio/src/AssetLoader.cpp`), the morph-attribute loop skips normal/tangent accessors **before** the `getElementType` type check:
```cpp
// The glTF normal and tangent data are ignored here, but honored in ResourceLoader.
if (atype == cgltf_attribute_type_normal || atype == cgltf_attribute_type_tangent) {
continue;
}
```
`getElementType` accepts only scalar/vec2/vec3/vec4 and rejects everything else, but morph normal/tangent accessors never reach it. `cgltf_validate()` imposes no semantic type constraint on morph attributes (it checks counts and sparse-index bounds only), so a `MAT4`-typed morph tangent accessor passes all validation.
Downstream, `TangentsJob` (`libs/gltfio/src/TangentsJob.cpp`) allocates `morphDeltas` as `new float3[vertexCount]` (3 floats/vertex) and calls:
```cpp
cgltf_accessor_unpack_floats(mtTangentsInfo, &morphDeltas[0].x, vertexCount * 3);
```
For a **sparse** accessor, `cgltf_accessor_unpack_floats`'s sparse pass writes `cgltf_num_components(type)` floats per element (16 for `MAT4`) at `writer_index * 16`, past the 3-float-per-vertex allocation — a heap out-of-bounds write whose offset (sparse index) and written values (sparse floats) are both attacker-controlled, with overflow size scaling with `vertexCount`.
### Reproduction (AddressSanitizer)
Crafted `.glb`: one primitive with a morph target whose TANGENT is a **sparse `MAT4`** accessor. Loaded through the default loader. Under an ASan build (`-DFILAMENT_ENABLE_ASAN_UBSAN=ON`), this produces a `heap-buffer-overflow WRITE` inside the tangent-space unpack during `createAsset`.
**Full PoC** — crafted-`.glb` builder script, standalone PoC, REPRO steps and ASan trace:
https://gist.github.com/ataberk-xyz/3c3dff3f869885f95bc3a66e04ba0e93
I'm happy to attach the crafted `.glb` or provide a self-contained `test_gltfio` case directly, whichever you prefer.
### Root cause / suggested direction
The morph NORMAL/TANGENT accessor type is simply never checked. Running the existing `getElementType` check on morph normal/tangent accessors (rejecting unsupported types such as matrix types) before the `continue` rejects the malformed accessor at load time, before it can reach `TangentsJob`. Valid `VEC3`/`VEC4` morph deltas are unaffected. (This is the one-line direction I had in the closed PR #10179, which also carried a `MorphTargetTangentInvalidTypeIsRejected` regression test.)
### Impact
Heap out-of-bounds write from a single untrusted, `cgltf_validate`-clean `.glb`/`.gltf`, during asset loading, before any GPU work, on the **default all-platform loader**. Attacker controls both the write offset and the written values. `libs/gltfio` also ships as the Maven artifact `com.google.android.filament:gltfio-android`, the glTF-loading backbone for Sceneform-derived / SceneView-based Android apps that load remote or marketplace-sourced models — a concrete untrusted-input path.
Contributor guide
Research direction
Start in libs/gltfio/src/AssetLoader.cpp at FAssetLoader::createPrimitive and inspect the morph-attribute loop and its getElementType check. Read libs/gltfio/src/TangentsJob.cpp to understand the downstream unpack, then run or add the MorphTargetTangentInvalidTypeIsRejected regression test. Done means malformed matrix-typed morph accessors are rejected while valid VEC3/VEC4 morph deltas still load.
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
- 84/100