google / google/filament

gltfio: UvMap out-of-bounds access in constrainMaterial and processShaderString via unclamped glTF texCoord index

Open
#10,202 1 comment 0 reactions 0 assignees View on GitHub
gltf security
Dominant language
C++
Stars
20.5k
Forks
2.3k
Avg merge
2d 13h
Merged PRs (30d)
90

Description

### Summary

`constrainMaterial` (`libs/gltfio/src/MaterialProvider.cpp`) indexes into a stack-allocated `UvMap retval {}` (8 elements, `std::array`) using UV channel indices taken directly from `MaterialKey` without any bounds check. The same unclamped indices are later used by `processShaderString` to index into the same `UvMap` and then into a 3-entry `uvstrings[]` array. Both accesses are reachable from a well-formed glTF file by setting any material texture's `texCoord` field to a value >= 8.

### Affected code

**Root cause - `AssetLoader.cpp` lines 1427, 1437–1447, 1455–1456:**

```cpp
MaterialKey matkey {
.baseColorUV = (uint8_t) baseColorTexture->texcoord,
.normalUV = (uint8_t) inputMat->normal_texture.texcoord,
.emissiveUV = (uint8_t) inputMat->emissive_texture.texcoord,
.aoUV = (uint8_t) inputMat->occlusion_texture.texcoord,
.transmissionUV = (uint8_t) trConfig.transmission_texture.texcoord,
.sheenColorUV = (uint8_t) shConfig.sheen_color_texture.texcoord,
.sheenRoughnessUV = (uint8_t) shConfig.sheen_roughness_texture.texcoord,
.volumeThicknessUV = (uint8_t) vlConfig.thickness_texture.texcoord,
.specularTextureUV = (uint8_t) spConfig.specular_texture.texcoord,
.specularColorTextureUV = (uint8_t) spConfig.specular_color_texture.texcoord,
// metallicRoughnessUV at line 1473, clearCoatUV/clearCoatRoughnessUV/clearCoatNormalUV at 1429-1433
};
```

`cgltf_int texcoord` is a plain `int`. `cgltf_validate` does not validate material texture `texcoord` values against any range. The cast to `uint8_t` silently truncates; values in [8, 255] are stored verbatim.

**Sink 1 - `MaterialProvider.cpp:72` (`constrainMaterial`):**

```cpp
UvMap retval {}; // std::array - 8 bytes on the stack

if (key->hasBaseColorTexture) {
retval[key->baseColorUV] = (UvSet) index++; // NO bounds check; OOB write if baseColorUV >= 8
}
if (key->hasMetallicRoughnessTexture && retval[key->metallicRoughnessUV] == UNUSED) {
retval[key->metallicRoughnessUV] = (UvSet) index++; // same
}
// ... identical pattern for all 14 UV fields
```

With `baseColorUV = 100`, line 77 writes 1 byte at `retval + 100` - 92 bytes past the end of the 8-byte stack array, into the caller's stack frame. Up to 14 separate OOB writes are possible from a single material with all texture types present.

**Sink 2 - `MaterialProvider.cpp:178` (`processShaderString`):**

```cpp
static const std::string uvstrings[] = { "vec2(0)", "getUV0()", "getUV1()" }; // 3 entries

const auto& normalUV = uvstrings[uvmap[config.normalUV]]; // OOB read if normalUV >= 8
const auto& baseColorUV = uvstrings[uvmap[config.baseColorUV]]; // same
const auto& metallicRoughnessUV = uvstrings[uvmap[config.metallicRoughnessUV]];
// ... 11 more UV fields, same pattern
```

`uvmap` is the correctly-sized 8-element result of `constrainMaterial`. `config.UV_field` is still the raw `uint8_t` from `MaterialKey`. If any UV field >= 8, `uvmap[config.UV_field]` reads past the 8-element array, and the garbage value is then used to index `uvstrings[]` (3 entries) - a second OOB read. This path is reached via `JitShaderProvider.cpp:381`.

### Trigger

A `.glb` or `.gltf` with any material texture `texCoord` >= 8:

```json
{
"materials": [{
"pbrMetallicRoughness": {
"baseColorTexture": { "index": 0, "texCoord": 100 }
}
}]
}
```

`cgltf_parse` accepts this without error; `cgltf_validate` does not check material `texCoord` values.

### Fix

Clamp all `texcoord` values at the point of `MaterialKey` construction in `AssetLoader.cpp`. This is the single root-cause fix and covers both `constrainMaterial` and `processShaderString`:

```diff
--- a/libs/gltfio/src/AssetLoader.cpp
+++ b/libs/gltfio/src/AssetLoader.cpp
@@ around line 1418
+ auto clampUV = [](cgltf_int tc) -> uint8_t {
+ return (uint8_t) std::min(std::max(tc, 0), (int)(UvMapSize - 1));
+ };
MaterialKey matkey {
...
- .baseColorUV = (uint8_t) baseColorTexture->texcoord,
+ .baseColorUV = clampUV(baseColorTexture->texcoord),
- .normalUV = (uint8_t) inputMat->normal_texture.texcoord,
+ .normalUV = clampUV(inputMat->normal_texture.texcoord),
- .emissiveUV = (uint8_t) inputMat->emissive_texture.texcoord,
+ .emissiveUV = clampUV(inputMat->emissive_texture.texcoord),
- .aoUV = (uint8_t) inputMat->occlusion_texture.texcoord,
+ .aoUV = clampUV(inputMat->occlusion_texture.texcoord),
- .transmissionUV = (uint8_t) trConfig.transmission_texture.texcoord,
+ .transmissionUV = clampUV(trConfig.transmission_texture.texcoord),
- .sheenColorUV = (uint8_t) shConfig.sheen_color_texture.texcoord,
+ .sheenColorUV = clampUV(shConfig.sheen_color_texture.texcoord),
- .sheenRoughnessUV = (uint8_t) shConfig.sheen_roughness_texture.texcoord,
+ .sheenRoughnessUV = clampUV(shConfig.sheen_roughness_texture.texcoord),
- .volumeThicknessUV = (uint8_t) vlConfig.thickness_texture.texcoord,
+ .volumeThicknessUV = clampUV(vlConfig.thickness_texture.texcoord),
- .specularTextureUV = (uint8_t) spConfig.specular_texture.texcoord,
+ .specularTextureUV = clampUV(spConfig.specular_texture.texcoord),
- .specularColorTextureUV = (uint8_t) spConfig.specular_color_texture.texcoord,
+ .specularColorTextureUV = clampUV(spConfig.specular_color_texture.texcoord),
};
// and the remaining assignments at lines 1429-1433, 1464, 1469, 1473
- .clearCoatUV = (uint8_t) ccConfig.clearcoat_texture.texcoord,
+ .clearCoatUV = clampUV(ccConfig.clearcoat_texture.texcoord),
// etc.
```

`UvMapSize` is already visible in `AssetLoader.cpp` (used at line 1094). The `clampUV` lambda requires `` which is already included.

Contributor guide

Open the contributing guide

Research direction

Start in libs/gltfio/src/AssetLoader.cpp at MaterialKey construction and review every material texture texCoord assignment, then trace the resulting values through constrainMaterial and processShaderString in MaterialProvider.cpp, reached from JitShaderProvider.cpp:381. Done means all assigned UV indices remain within the existing UvMap bounds and a glTF texture texCoord of 8 or greater no longer produces out-of-bounds access.

Written by the indexing model from the issue text.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.