google / google/filament

filameshio: heap OOB write in MeshReader compressed vertex path when UV1 absent but sizes.uv1 non-zero

Open Beginner friendly
#10,200 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

`MeshReader::loadMeshFromBuffer` (non-interleaved COMPRESSION path, `libs/filameshio/src/MeshReader.cpp`) allocates the `uncompressed` output buffer sized for `32 * vertexCount` bytes when `hasUV1 = false`, then decodes a UV1 stream into that buffer if `CompressionHeader.uv1 != 0` - writing `4 * vertexCount` bytes past the end of the allocation.

### Affected code

`libs/filameshio/src/MeshReader.cpp`, non-interleaved COMPRESSION path, lines 368-453.

**Buffer allocation (lines 368-393):**

```cpp
constexpr uint32_t uintmax = std::numeric_limits::max();
const bool hasUV1 = header.offsetUV1 != uintmax && header.strideUV1 != uintmax;

size_t vertexSize = sizeof(half4) + sizeof(short4) + sizeof(ubyte4) + sizeof(ushort2)
+ (hasUV1 ? sizeof(ushort2) : 0);
// vertexSize = 32 bytes when hasUV1 == false (no UV1 space in allocation)
size_t uncompressedSize = vertexSize * vertexCount;
void* uncompressed = malloc(uncompressedSize);
```

**CompressionHeader parsed separately (lines 415-416):**

```cpp
CompressionHeader sizes;
memcpy(&sizes, vertexData, sizeof(CompressionHeader));
// sizes.uv1 comes from the file - never cross-validated against hasUV1
```

**Decode loop (lines 432-453):**

```cpp
uint8_t* dstdata = (uint8_t*) uncompressed;

err |= decode(dstdata, vertexCount, sizeof(half4), srcdata, sizes.positions);
srcdata += sizes.positions;
dstdata += sizeof(half4) * vertexCount; // dstdata at offset 16*VC

err |= decode(dstdata, vertexCount, sizeof(short4), srcdata, sizes.tangents);
srcdata += sizes.tangents;
dstdata += sizeof(short4) * vertexCount; // dstdata at offset 24*VC

err |= decode(dstdata, vertexCount, sizeof(ubyte4), srcdata, sizes.colors);
srcdata += sizes.colors;
dstdata += sizeof(ubyte4) * vertexCount; // dstdata at offset 28*VC

err |= decode(dstdata, vertexCount, sizeof(ushort2), srcdata, sizes.uv0);
// dstdata NOT advanced after UV0 decode

if (sizes.uv1) { // checked against CompressionHeader, NOT hasUV1
srcdata += sizes.uv0;
dstdata += sizeof(ushort2) * vertexCount; // dstdata now at offset 32*VC = end of allocation
err |= decode(dstdata, vertexCount, sizeof(ushort2), srcdata, sizes.uv1);
// writes 4*vertexCount bytes past end of uncompressed
}
```

`hasUV1` (from the vertex layout header) and `CompressionHeader.uv1` (from the compression header) are two independent fields with no cross-validation. The bounds check at line 424 (`compressedSum <= verticesSize`) guards the compressed input bytes only - it does not protect the destination buffer.

### Trigger conditions

A `.filamesh` file with:
1. `header.flags = COMPRESSION` (bit 2 set, bit 0 clear - non-interleaved path)
2. `header.offsetUV1 = 0xFFFFFFFF` or `header.strideUV1 = 0xFFFFFFFF` - forces `hasUV1 = false`
3. `CompressionHeader.uv1 != 0` - any non-zero value (e.g. a valid meshopt-compressed UV1 block)
4. `sizeof(CompressionHeader) + sizes.positions + sizes.tangents + sizes.colors + sizes.uv0 + sizes.uv1 <= header.vertexSize` - satisfies the source-side check at line 424

### Impact

`4 * vertexCount` bytes of meshopt-decoded data are written past the end of the `uncompressed` heap allocation, corrupting adjacent heap contents. `vertexCount` is read directly from the file header with only a multiplication overflow check (line 390), so the write size is attacker-controlled.

### Suggested fix

Reject the file early when the compressed UV1 stream is present but UV1 is absent from the vertex layout:

```diff
--- a/libs/filameshio/src/MeshReader.cpp
+++ b/libs/filameshio/src/MeshReader.cpp
@@ -414,6 +414,13 @@ MeshReader::Mesh MeshReader::loadMeshFromBuffer(...) {
} else {
CompressionHeader sizes;
memcpy(&sizes, vertexData, sizeof(CompressionHeader));
+
+ if (!hasUV1 && sizes.uv1 != 0) {
+ utils::slog.e << "Compressed UV1 stream present but UV1 absent from vertex header." << utils::io::endl;
+ free(uncompressed);
+ engine->destroy(mesh.vertexBuffer);
+ engine->destroy(mesh.indexBuffer);
+ return {};
+ }
+
size_t compressedSum = sizeof(CompressionHeader) +
```

Contributor guide

Open the contributing guide

Research direction

Start in libs/filameshio/src/MeshReader.cpp at MeshReader::loadMeshFromBuffer, especially the non-interleaved COMPRESSION path around lines 368-453. Trace how hasUV1 and CompressionHeader.uv1 affect allocation and decoding; done means an input with a compressed UV1 stream but no UV1 layout is rejected before the out-of-bounds decode.

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.