From 1df710186d778ecf694c61afebe64e983280b771 Mon Sep 17 00:00:00 2001 From: Cyl Date: Sat, 4 Apr 2026 10:23:21 +0800 Subject: [PATCH] fix: add bounds checking in normalizeSkinningWeights to prevent heap OOB write The normalize lambda in normalizeSkinningWeights trusts the accessor's count field without validating it against the actual buffer size. A maliciously crafted glTF file can set an arbitrarily large count with a small buffer, causing out-of-bounds heap writes during weight normalization. This patch adds the following safety checks: - Null pointer validation for buffer_view, buffer, and data pointers - Offset validation to ensure accessor offset is within buffer bounds - Safe count computation based on available buffer bytes and stride - Clamping of accessor count to prevent out-of-bounds access The fix gracefully handles malformed inputs by clamping the iteration count and logging a warning, rather than crashing or corrupting memory. --- libs/gltfio/src/ResourceLoader.cpp | 40 ++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/libs/gltfio/src/ResourceLoader.cpp b/libs/gltfio/src/ResourceLoader.cpp index 175b3f3b5d33..c2b009fe62e7 100644 --- a/libs/gltfio/src/ResourceLoader.cpp +++ b/libs/gltfio/src/ResourceLoader.cpp @@ -163,9 +163,45 @@ inline void normalizeSkinningWeights(cgltf_data const* gltf) { LOG(WARNING) << "Cannot normalize weights, unsupported attribute type."; return; } + if (!data->buffer_view || !data->buffer_view->buffer || + !data->buffer_view->buffer->data) { + LOG(WARNING) << "Cannot normalize weights, missing buffer data."; + return; + } + const cgltf_size bufferSize = data->buffer_view->buffer->size; + const cgltf_size viewOffset = data->buffer_view->offset; + const cgltf_size accessorOffset = data->offset; + const cgltf_size totalOffset = viewOffset + accessorOffset; + + // Validate that the starting offset is within the buffer. + if (totalOffset >= bufferSize) { + LOG(WARNING) << "Cannot normalize weights, accessor offset exceeds buffer size."; + return; + } + + const cgltf_size availableBytes = bufferSize - totalOffset; + const cgltf_size stride = data->stride; + + // Compute the maximum number of elements that fit within bounds. + // Each element requires at least sizeof(float4) bytes, and elements + // are separated by 'stride' bytes. + cgltf_size maxCount = 0; + if (stride > 0 && availableBytes >= sizeof(float4)) { + // The last element needs sizeof(float4) bytes; preceding elements need 'stride' each. + maxCount = 1 + (availableBytes - sizeof(float4)) / stride; + } + + cgltf_size safeCount = data->count; + if (safeCount > maxCount) { + LOG(WARNING) << "Skinning weights accessor count (" << safeCount + << ") exceeds buffer capacity (" << maxCount + << "), clamping to prevent out-of-bounds access."; + safeCount = maxCount; + } + uint8_t* bytes = (uint8_t*) data->buffer_view->buffer->data; - bytes += data->offset + data->buffer_view->offset; - for (cgltf_size i = 0, n = data->count; i < n; ++i, bytes += data->stride) { + bytes += totalOffset; + for (cgltf_size i = 0, n = safeCount; i < n; ++i, bytes += stride) { float4* weights = (float4*) bytes; const float sum = weights->x + weights->y + weights->z + weights->w; *weights /= sum;