Reject vertex/triangle counts whose allocation size would wrap 32 bits - #3
Merged
Conversation
Match the surrounding OOM error paths, which all call _ctmClearMesh before returning.
…ze_t
The previous guard rejected counts above size_t max / 32, so on a 64-bit
size_t it never fired. The mesh allocations in ctmLoadCustom multiply by a
size_t sizeof and are safe there, but several count * stride products are
computed in 32-bit CTMuint and wrap regardless of size_t width:
- _ctmAllocateFloatMaps: size was CTMuint, so aChannels * 4 * mVertexCount
(up to 16 bytes/vertex) truncated to 32 bits before malloc/memset, while
the decoder then wrote mVertexCount * channels floats -> heap overflow.
- stream.c packed int/float temp buffers: malloc(aCount * aSize * 4), all
32-bit operands.
On 64-bit these sit behind multi-GB sibling allocations, but under Linux
memory overcommit those can succeed, making the truncated map write reachable
(not merely a large-allocation DoS).
Bounding both counts by 0xFFFFFFFF / 32 keeps count * stride (stride <= 16)
below 2^32 on every platform, closing the CTMuint products at the source; the
wasm32 (32-bit size_t) overflow this PR targets stays closed. Also compute the
float-map allocation size in size_t so it is correct independent of the guard.
Grantim
approved these changes
Sep 8, 2026
This was referenced Sep 8, 2026
Bump OpenCTM to fix heap overflows on crafted .ctm files (S9S disclosure)
MeshInspector/MeshLib#6800
Merged
Fedr
added a commit
that referenced
this pull request
Sep 8, 2026
…#4) Fork point (upstream v1.0.3), library-only scope, CMake build and export macro, ctmRearrangeTriangles, ctmSaveCustom progress/cancel callback, empty-mesh support, noexcept in the C++ wrapper, the #2/#3 loader hardening, and file-format compatibility notes. README.txt is kept verbatim as the upstream change log.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to the
_ctmStreamReadSTRINGheap-overflow fix (#2), addressing the secondary item flagged in the same coordinated disclosure by Kamal Sentassi of S9S Security Research: the file-declared vertex and triangle counts drivemallocdirectly.ctmLoadCustomreadsmVertexCountandmTriangleCountas 32-bit values straight from the file, then allocatescount * stridefor the mesh arrays and for every temporary buffer in the RAW/MG1/MG2 decoders. On a platform wheresize_tis 32-bit — the wasm32 target MeshLib builds OpenCTM for under Emscripten — that multiplication wraps, producing an undersized buffer that the decoder then overruns. This is the same integer-overflow-to-heap-write class as #2, reachable from the publicctmLoadon a single crafted file.Independently of
size_twidth, somecount * strideproducts are computed in 32-bitCTMuintand wrap on any platform:_ctmAllocateFloatMapstruncatedaChannels * sizeof(CTMfloat) * mVertexCountinto aCTMuint size, and the packed int/float temp buffers instream.callocateaCount * aSize * 4with 32-bit operands. On a 64-bitsize_tthese sit behind the multi-GB mesh allocations, but under Linux memory overcommit those can succeed, so the truncated float-map write is reachable there too — not merely a large-allocation DoS.Fix (
lib/openctm.c)Immediately after the counts are read, reject any
mVertexCountormTriangleCountabove0xFFFFFFFF / 16, 16 bytes (4 floats per vertex) being the largest real stride. The file is rejected withCTM_BAD_FORMAT. A single up-front check covers every count-driven allocation in the loader; legitimately large-but-non-wrapping counts still fall through to the existingCTM_OUT_OF_MEMORYpath.The bound is
UINT_MAX / 16, notSIZE_MAX / 16: the counts are 32-bit, andcount * 3,count * 4and thestream.ctemp buffer sizes are still computed in 32-bitCTMuinton every platform, socount * stridehas to stay below 2^32 regardless ofsize_twidth. On 32-bitsize_tthe two bounds coincide anyway. On 64-bit this essentially matches the pre-existing effective ceiling: the 4-channel attribute-map allocation and packed buffers already wrapped their 32-bit products at 2^28 elements; only meshes without attribute maps between 2^28 and ~3.6e8 elements loaded before and are rejected now.The float-map allocation size is also computed in
size_tso it is correct by construction, independent of the guard.Also (
lib/compressMG1.c)_ctmUncompressMesh_MG1leaked its temporaryindicesbuffer when the packed-int read of theINDXchunk failed. Freed on that path, matching the sibling error paths.Verification
Built the library for
i686(32-bitsize_t, matching wasm32) and for x86-64, driven through the publicctmLoad.0x15555556vertices (whosecount * 4 * 3wraps to 8 bytes) segfaults as the decoder writes past the 8-byte buffer.CTM_BAD_FORMATand no crash.Notes
_ctmClearMeshcall added on themVerticesfailure path is a consistency/future-proofing measure —mIndices/mNormalsare stillNULLat that point, so it frees nothing new; it is not itself a leak fix. It matches the sibling error paths andmFileCommentremains owned by the context untilctmFreeContext.mUVMapCount/mAttribMapCountdrives a long allocation loop rather than a single wrapping multiply, so it self-limits viaCTM_OUT_OF_MEMORY; left as-is. The other error-path leaks flagged in the original report were addressed in Fix heap-buffer-overflow in _ctmStreamReadSTRING and stop parsing on string read errors #2.