From 11b020b2661b26c8d257a858040a54aec9baec04 Mon Sep 17 00:00:00 2001 From: Sebastian Josue Alba Vives Date: Wed, 1 Apr 2026 16:35:25 -0600 Subject: [PATCH] Fix heap-buffer-overflow in glTF decoder accessor reads Add bounds validation to all glTF accessor data copy functions to prevent out-of-bounds heap reads when processing crafted .glb/.gltf files with malicious byteOffset, byteLength, or byteStride values. The glTF decoder trusted accessor/bufferView fields from the input file without validating them against the actual buffer size. A crafted .glb file could cause memcpy to read past the end of the allocated buffer, leading to heap-buffer-overflow (confirmed via AddressSanitizer). Affected functions: - TinyGltfUtils::CopyDataAsFloatImpl() in tiny_gltf_utils.h - CopyDataAsUint32() in gltf_decoder.cc - CopyDataAs() (both specializations) in gltf_decoder.cc - CopyDataFromBufferView() in gltf_decoder.cc --- src/draco/io/gltf_decoder.cc | 80 ++++++++++++++++++++++++++++++---- src/draco/io/tiny_gltf_utils.h | 23 ++++++++-- 2 files changed, 91 insertions(+), 12 deletions(-) diff --git a/src/draco/io/gltf_decoder.cc b/src/draco/io/gltf_decoder.cc index 2ea71ec8a..69fb52179 100644 --- a/src/draco/io/gltf_decoder.cc +++ b/src/draco/io/gltf_decoder.cc @@ -160,14 +160,33 @@ StatusOr> CopyDataAsUint32( const tinygltf::Buffer &buffer = model.buffers[buffer_view.buffer]; - const uint8_t *const data_start = - buffer.data.data() + buffer_view.byteOffset + accessor.byteOffset; const int byte_stride = accessor.ByteStride(buffer_view); const int component_size = tinygltf::GetComponentSizeInBytes(accessor.componentType); const int num_components = TinyGltfUtils::GetNumComponentsForType(accessor.type); - const int num_elements = accessor.count * num_components; + + // Validate that accessor data fits within the buffer. + const size_t start_offset = + static_cast(buffer_view.byteOffset) + accessor.byteOffset; + if (accessor.count <= 0 || component_size <= 0 || num_components <= 0 || + byte_stride <= 0) { + return Status(Status::DRACO_ERROR, + "Invalid accessor or buffer view parameters."); + } + const size_t last_element_end = + start_offset + + static_cast(accessor.count - 1) * byte_stride + + static_cast(num_components) * component_size; + if (last_element_end > buffer.data.size()) { + return Status(Status::DRACO_ERROR, + "Accessor data exceeds buffer bounds."); + } + + const uint8_t *const data_start = buffer.data.data() + start_offset; + + const size_t num_elements = + static_cast(accessor.count) * num_components; std::vector output; output.resize(num_elements); @@ -225,17 +244,34 @@ StatusOr> CopyDataAs(const tinygltf::Model &model, const tinygltf::Buffer &buffer = model.buffers[buffer_view.buffer]; - const uint8_t *const data_start = - buffer.data.data() + buffer_view.byteOffset + accessor.byteOffset; const int byte_stride = accessor.ByteStride(buffer_view); const int component_size = tinygltf::GetComponentSizeInBytes(accessor.componentType); + const int num_components = + TinyGltfUtils::GetNumComponentsForType(accessor.type); + + // Validate that accessor data fits within the buffer. + const size_t start_offset = + static_cast(buffer_view.byteOffset) + accessor.byteOffset; + if (accessor.count <= 0 || component_size <= 0 || num_components <= 0 || + byte_stride <= 0) { + return Status(Status::DRACO_ERROR, + "Invalid accessor or buffer view parameters."); + } + const size_t last_element_end = + start_offset + + static_cast(accessor.count - 1) * byte_stride + + static_cast(num_components) * component_size; + if (last_element_end > buffer.data.size()) { + return Status(Status::DRACO_ERROR, + "Accessor data exceeds buffer bounds."); + } + + const uint8_t *const data_start = buffer.data.data() + start_offset; std::vector output; output.resize(accessor.count); - const int num_components = - TinyGltfUtils::GetNumComponentsForType(accessor.type); int out_index = 0; const uint8_t *data = data_start; for (int i = 0; i < accessor.count; ++i) { @@ -273,12 +309,29 @@ StatusOr> CopyDataAs(const tinygltf::Model &model, const tinygltf::Buffer &buffer = model.buffers[buffer_view.buffer]; - const uint8_t *const data_start = - buffer.data.data() + buffer_view.byteOffset + accessor.byteOffset; const int byte_stride = accessor.ByteStride(buffer_view); const int component_size = tinygltf::GetComponentSizeInBytes(accessor.componentType); + // Validate that accessor data fits within the buffer. + const size_t start_offset = + static_cast(buffer_view.byteOffset) + accessor.byteOffset; + if (accessor.count <= 0 || component_size <= 0 || num_components <= 0 || + byte_stride <= 0) { + return Status(Status::DRACO_ERROR, + "Invalid accessor or buffer view parameters."); + } + const size_t last_element_end = + start_offset + + static_cast(accessor.count - 1) * byte_stride + + static_cast(num_components) * component_size; + if (last_element_end > buffer.data.size()) { + return Status(Status::DRACO_ERROR, + "Accessor data exceeds buffer bounds."); + } + + const uint8_t *const data_start = buffer.data.data() + start_offset; + std::vector output; output.resize(accessor.count); @@ -310,6 +363,15 @@ Status CopyDataFromBufferView(const tinygltf::Model &model, int buffer_view_id, } const tinygltf::Buffer &buffer = model.buffers[buffer_view.buffer]; + + // Validate that buffer view data fits within the buffer. + const size_t end_offset = + static_cast(buffer_view.byteOffset) + buffer_view.byteLength; + if (end_offset > buffer.data.size()) { + return ErrorStatus( + "Buffer view byteOffset + byteLength exceeds buffer size."); + } + const uint8_t *const data_start = buffer.data.data() + buffer_view.byteOffset; data->resize(buffer_view.byteLength); diff --git a/src/draco/io/tiny_gltf_utils.h b/src/draco/io/tiny_gltf_utils.h index a536a70fb..187ed73d4 100644 --- a/src/draco/io/tiny_gltf_utils.h +++ b/src/draco/io/tiny_gltf_utils.h @@ -101,16 +101,33 @@ class TinyGltfUtils { const tinygltf::Buffer &buffer = model.buffers[buffer_view.buffer]; - const unsigned char *const data_start = - buffer.data.data() + buffer_view.byteOffset + accessor.byteOffset; const int byte_stride = accessor.ByteStride(buffer_view); const int component_size = tinygltf::GetComponentSizeInBytes(accessor.componentType); + const int num_components = GetNumComponentsForType(accessor.type); + + // Validate that accessor data fits within the buffer. + const size_t start_offset = + static_cast(buffer_view.byteOffset) + accessor.byteOffset; + if (accessor.count <= 0 || component_size <= 0 || num_components <= 0 || + byte_stride <= 0) { + return Status(Status::DRACO_ERROR, + "Invalid accessor or buffer view parameters."); + } + const size_t last_element_end = + start_offset + + static_cast(accessor.count - 1) * byte_stride + + static_cast(num_components) * component_size; + if (last_element_end > buffer.data.size()) { + return Status(Status::DRACO_ERROR, + "Accessor data exceeds buffer bounds."); + } + + const unsigned char *const data_start = buffer.data.data() + start_offset; std::vector output; output.resize(accessor.count); - const int num_components = GetNumComponentsForType(accessor.type); const unsigned char *data = data_start; for (int i = 0; i < accessor.count; ++i) { T values;