Repository navigation
OpenFBX: out-of-bounds access in FBX geometry post-processing from unchecked polygon and material indices #120
Description
Hello, and thank you for maintaining this project.
I found a likely memory-safety issue by static source review (no runtime exploit was developed). The technical details are inline below so this report is self-contained, with a full write-up linked within.
Affected component: OpenFBX, a single-file C++ FBX model loader. Affected component: GeometryDataImpl::postprocess in src/ofbx.cpp, reached from the public ofbx::load API during geometry post-processing.
Tested version(s): Release v0.9, reviewed at commit ef492927cefc16ad8733ec08bcc8df5434dbb1ae, and default-branch HEAD at commit 4d4a45a0ed8f357d11ff83904b087530547a4071 (master, 2026-05-16). Both carry the same code. OpenFBX ships as a vendored source pair (ofbx.cpp, ofbx.h), so consumers embed the affected revision directly.
Details:
Two memory-safety defects in the same function, GeometryDataImpl::postprocess in src/ofbx.cpp, both reachable from the public entry ofbx::load(const u8* data, usize size, u16 flags, JobProcessor = nullptr, void* = nullptr) under the default configuration (flags = 0, default synchronous job processor installed at ofbx.cpp:3720). A crafted .fbx passed to ofbx::load is parsed into deferred jobs that fill geometry arrays from the file (ofbx.cpp:3893), then post-process jobs run GeometryDataImpl::postprocess() (ofbx.cpp:4135 and :4136, function at :1438). No authentication precedes the parse. VulnCheck may split these into separate CVEs.
FINDING 1. Weakness: CWE-125 Out-of-bounds Read. When a layer element declares ByVertex mapping with IndexToDirect references, the templated postprocess(T& attr) (ofbx.cpp:1404) remaps the attribute index array with remapped[i] = attr.indices[decodeIndex(positions.indices[i])] at ofbx.cpp:1414. positions.indices holds PolygonVertexIndex values from the file, decodeIndex (ofbx.cpp:278) returns a non-negative but unbounded value, and attr.indices is the layer's own index array (for normals, NormalsIndex), whose length is independent and can be 1. Array::operator[] (ofbx.cpp:29) performs no bound check, and nothing checks decodeIndex(positions.indices[i]) < attr.indices.size(). Attacker model: a single crafted FBX with a short NormalsIndex and a PolygonVertexIndex value larger than that length. Impact: out-of-bounds read inside ofbx::load, with the leaked bytes written into the decoded index array and a large offset crashing the loader. Fix: reject a decoded index that is negative or not less than attr.indices.size() before the subscript. Sibling contrast: the adjacent BY_POLYGON branch validates sizes at ofbx.cpp:1421 and the partition accessor bounds its index at ofbx.cpp:1395.
FINDING 2. Weakness: CWE-125 Out-of-bounds Read with a downstream CWE-787 Out-of-bounds Write. The material branch of GeometryDataImpl::postprocess() computes max_partition over signed material values and sizes partitions to max_partition + 1 (ofbx.cpp:1471), then for each polygon reads u32 material_index = materials[polygon_idx] at ofbx.cpp:1480 and uses it as partitions[material_index] at ofbx.cpp:1481, writing through it with partition.polygons.push_back(...) at ofbx.cpp:1482. materials is the LayerElementMaterial Materials array (ofbx.cpp:2484 to :2488), with file-controlled length and values. There is no check that polygon_idx < materials.size(), so a geometry with more polygons than material entries reads materials out of bounds. Separately, a negative material value (for example -1) never raises max_partition, yet converts to a large u32, so partitions[0xFFFFFFFF] is a wild reference and the following push_back is an out-of-bounds write. Attacker model: a crafted FBX with a ByPolygon LayerElementMaterial whose Materials array is shorter than the polygon count or contains a negative value. Impact: out-of-bounds read of the materials array and a wild write through the partition array, memory corruption and a crash during load. Fix: reject negative material values, check polygon_idx < materials.size(), and check material_index < partitions.size() before the subscript.
Verification. Source-only for this submission. The full chain was confirmed in the released source at the two pinned commits, from ofbx::load through the parse and post-process jobs into the two sinks, including the missing bounds, the attacker-controlled fields, and default-config reachability. No request was executed against any live deployment and no runtime harness was run in this phase, so runtime effects (exact fault offset, read-back, control of the write) are inferred from the source path and not claimed beyond that.
Detailed write-up: https://hackmd.io/@haindpwn/openfbx-geometry-index-remap-material-map-oob
Full technical write-up (trace, PoC builder, suggested fix): https://hackmd.io/@haindpwn/openfbx-geometry-index-remap-material-map-oob`
Reported by HaiND (Nguyen Dinh Hai), Post and Telecommunication Institute of Technology, github.com/Haind03.
Activity