filameshio: heap OOB write in MeshReader compressed vertex path when UV1 absent but sizes.uv1 non-zero
- 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
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