google / google/filament

gltfio: heap out-of-bounds write in `TangentsJob` via unvalidated `MAT4`-typed morph normal/tangent accessor (type confusion)

Open Beginner friendly
#10,194 1 comment 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

### 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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.