Skip to content

fix: integer overflow in face/point count multiplications → heap overflow#1166

Open
mohammadmseet-hue wants to merge 1 commit intogoogle:mainfrom
mohammadmseet-hue:fix/integer-overflow-face-point-count
Open

fix: integer overflow in face/point count multiplications → heap overflow#1166
mohammadmseet-hue wants to merge 1 commit intogoogle:mainfrom
mohammadmseet-hue:fix/integer-overflow-face-point-count

Conversation

@mohammadmseet-hue
Copy link
Copy Markdown

Summary

Multiple locations in the Unity plugin, JavaScript/WASM decoder wrapper, and Maya plugin compute buffer sizes using num_faces * 3 or num_points * N with attacker-controlled values from the Draco bitstream, without checking for integer overflow. When the product wraps to a small value, a tiny buffer is allocated and subsequent loops write the full (unwrapped) count of entries, causing a heap buffer overflow.

Severity

Critical — Heap buffer overflow from crafted Draco files. Affects:

  • Unity games loading user-provided 3D models
  • Web applications using the Draco WASM decoder (Three.js, Google Model Viewer)
  • Maya plugin loading untrusted meshes

Vulnerable Code

1. JavaScript/WASM Decoder (decoder_webidl_wrapper.cc:178)

const uint32_t num_faces = m.num_faces();
if (num_faces * 3 * sizeof(T) != out_size) {  // Overflow breaks this check

On WASM32, sizeof(T)=4, so 357913942 * 3 * 4 = 4294967304 wraps to 8. If out_size=8, the check passes and the loop writes ~1.4B entries into an 8-byte buffer.

2. Unity Plugin (draco_unity_plugin.cc:239, 335, 343, 358, 373, 393)

int *const temp_indices = new int[m->num_faces() * 3];  // Overflow → tiny alloc
unity_mesh->position = new float[in_mesh->num_points() * 3];
unity_mesh->normal = new float[in_mesh->num_points() * 3];
unity_mesh->color = new float[in_mesh->num_points() * 4];
unity_mesh->texcoord = new float[in_mesh->num_points() * 2];

3. Maya Plugin (draco_maya_plugin.cc:23)

out_mesh->faces = new int[num_faces * 3];  // Overflow → tiny alloc

Trigger

num_faces = 1431655766 (just over UINT32_MAX / 3):

  • 1431655766 * 3 = 4294967298 → overflows 32-bit to 2
  • new int[2] allocates 8 bytes
  • Loop writes 1431655766 * 3 = ~4.3 billion entries → heap overflow

Note

The core decoder (mesh/corner_table.cc:63) does have an overflow check for num_faces * 3. But the binding/extraction layer (Unity, WASM, Maya) where decoded data is copied to caller buffers has no overflow protection. Existing fuzz targets do not cover these code paths.

Fix

Added overflow checks before each unchecked multiplication in the Unity, WASM, and Maya plugins.

Multiple locations in the Unity plugin, JavaScript/WASM wrapper, and Maya
plugin compute buffer sizes as num_faces * 3 or num_points * N without
checking for integer overflow. When the product overflows (wraps to a
small value), a tiny buffer is allocated and subsequent loops write the
full count of entries, causing a heap buffer overflow.

This is exploitable on 32-bit platforms (WASM32, 32-bit Unity builds)
where the overflow threshold is practical (e.g., num_faces > ~1.4 billion
causes num_faces * 3 to wrap).

Affected locations:
- unity/draco_unity_plugin.cc: GetMeshIndices, DecodeDracoMeshStep2
  (indices, positions, normals, colors, texcoords)
- javascript/emscripten/decoder_webidl_wrapper.cc: GetTrianglesArray
- maya/draco_maya_plugin.cc: decode_faces

Fix: add overflow checks before each unchecked multiplication.
@mohammadmseet-hue
Copy link
Copy Markdown
Author

ASan Confirmation

Built with -fsanitize=address and confirmed the heap buffer overflow:

PoC 1: Simple overflow pattern

==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x502000000018
WRITE of size 4 at 0x502000000018 thread T0
    #0 in main poc_overflow.cc:93

0x502000000018 is located 0 bytes after 8-byte region [0x502000000010,0x502000000018)
allocated by thread T0 here:
    #0 in operator new[](unsigned long)
    #1 in main poc_overflow.cc:87

SUMMARY: AddressSanitizer: heap-buffer-overflow poc_overflow.cc:93 in main

PoC 2: Realistic pattern using draco::Mesh + memcpy (exact code from draco_unity_plugin.cc:242)

==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x503000000058
WRITE of size 12 at 0x503000000058 thread T0
    #0 in memcpy
    #1 in main poc_realistic.cc:83

0x503000000058 is located 0 bytes after 24-byte region [0x503000000040,0x503000000058)
allocated by thread T0 here:
    #0 in operator new[](unsigned long)
    #1 in main poc_realistic.cc:76

SUMMARY: AddressSanitizer: heap-buffer-overflow in memcpy

Both PoCs use the exact memcpy(temp_indices + face_id.value() * 3, face.data(), sizeof(int) * 3) pattern from draco_unity_plugin.cc:242-243.

@mohammadmseet-hue
Copy link
Copy Markdown
Author

ASan proof for the ply_reader.cc heap-buffer-overflow

Verified on b283ba3 (HEAD). PlyReader::ParseElementData (src/draco/io/ply_reader.cc:204-208) reads an attacker-controlled list length (num_entries, up to 2^32-1 from a PLY property list header) and multiplies by prop.data_type_num_bytes() (up to 8), then feeds the result into std::vector::insert / DecoderBuffer::Advance with no check against the remaining input buffer.

Reachability

Public tool: draco_encoder -i attacker.ply. Also reaches draco::ReadMeshFromFile used by downstream glTF pipelines.

109-byte PoC (evil.ply)

ply
format binary_little_endian 1.0
element face 1
property list uint32 double vertex_indices
end_header
<4 bytes: FF FF FF FF>

Header declares one face whose vertex list has 0xFFFFFFFF double entries (~34 GB). ASan fires on the first OOB byte.

Build

mkdir build-asan && cd build-asan
CC=clang CXX=clang++ cmake -DCMAKE_CXX_FLAGS="-fsanitize=address,undefined -fno-omit-frame-pointer -g -O1" \
                           -DCMAKE_C_FLAGS="-fsanitize=address,undefined -fno-omit-frame-pointer -g -O1" ..
make -j draco_encoder
./draco_encoder -i evil.ply

ASan output

==1090379==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x50b0000000ad
READ of size 1 at 0x50b0000000ad thread T0
    #8  std::vector<unsigned char,...>::_M_range_insert<char const*>
    #10 draco::PlyReader::ParseElementData  src/draco/io/ply_reader.cc:206:20
    #11 draco::PlyReader::ParsePropertiesData   src/draco/io/ply_reader.cc:177:12
    #12 draco::PlyReader::Read                  src/draco/io/ply_reader.cc:71:8
    #13 draco::PlyDecoder::DecodeInternal()     src/draco/io/ply_decoder.cc:71:3
    #14 draco::PlyDecoder::DecodeFromBuffer     src/draco/io/ply_decoder.cc:66:10
    #15 draco::PlyDecoder::DecodeFromFile       src/draco/io/ply_decoder.cc:54:10
    #17 draco::ReadMeshFromFile                 src/draco/io/mesh_io.cc:79:5
    #19 main                                    src/draco/tools/draco_encoder.cc:276:23

0x50b0000000ad is located 0 bytes after 109-byte region [0x50b000000040,0x50b0000000ad)
allocated by thread T0 here:
    #6 draco::StdioFileReader::ReadFileToBuffer   src/draco/io/stdio_file_reader.cc:64:11
    #7 draco::ReadFileToBuffer                    src/draco/io/file_utils.cc:84:23

SUMMARY: AddressSanitizer: heap-buffer-overflow ply_reader.cc:206:20 in draco::PlyReader::ParseElementData

Info-disclosure angle

The OOB bytes are copied into the mesh vector and re-encoded into the output .drc, so any service that accepts .ply uploads, converts them with draco_encoder, and returns the .drc to the uploader becomes a Heartbleed-style heap-byte oracle.

Fix

Match this PR: reject negative num_entries and clamp num_bytes_to_read against buffer->remaining_size() before the insert/Advance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant