From 51f19b432917059fb88db32eddd045da286bd188 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20M=C3=BCller?= <34514239+appgurueu@users.noreply.github.com> Date: Sat, 18 Oct 2025 20:00:37 +0200 Subject: [PATCH] Fix overly strict bounds check in tiniergltf (#16590) This makes sure that models exported by Goxel are not falsely rejected. It applies to exporters using strides more broadly. A workaround is to add padding to the buffer and buffer view. --- lib/tiniergltf/tiniergltf.hpp | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/lib/tiniergltf/tiniergltf.hpp b/lib/tiniergltf/tiniergltf.hpp index 81ac10ec8..bbaeaa476 100644 --- a/lib/tiniergltf/tiniergltf.hpp +++ b/lib/tiniergltf/tiniergltf.hpp @@ -31,6 +31,22 @@ static inline void check(bool cond) { throw std::runtime_error("invalid glTF"); } +// Unsigned arithmetic helpers with wraparound checks + +template +static inline T checkedAdd(T a, T b) { + T c = std::numeric_limits::max() - a; + check(b <= c); + return a + b; +} + +template +static inline T checkedMul(T a, T b) { + T prod = a * b; + check(a == 0 || prod / a == b); + return prod; +} + template static inline void checkIndex(const std::optional> &vec, const std::optional &i) { @@ -1241,10 +1257,8 @@ struct GlTF { checkForall(bufferViews, [&](const BufferView &view) { check(buffers.has_value()); const Buffer &buf = buffers->at(view.buffer); - // Be careful because of possible integer overflows. - check(view.byteOffset < buf.byteLength); - check(view.byteLength <= buf.byteLength); - check(view.byteOffset <= buf.byteLength - view.byteLength); + // View must fit into the buffer + check(checkedAdd(view.byteOffset, view.byteLength) <= buf.byteLength); }); const auto checkAccessor = [&](const auto &accessor, @@ -1252,10 +1266,13 @@ struct GlTF { const BufferView &view = bufferViews->at(bufferView); if (view.byteStride.has_value()) check(*view.byteStride % accessor.componentSize() == 0); - check(byteOffset < view.byteLength); - // Use division to avoid overflows. - const auto effective_byte_stride = view.byteStride.value_or(accessor.elementSize()); - check(count <= (view.byteLength - byteOffset) / effective_byte_stride); + + const std::size_t effective_byte_stride = view.byteStride.value_or(accessor.elementSize()); + // Accessor must fit into the buffer view: The last element must be fully in bounds. + // Want: (count-1) * effective_byte_stride + accessor.elementSize() + byteOffset <= view.byteLength + // Be careful to avoid wraparound. + check(checkedAdd(checkedMul(count - 1, effective_byte_stride), + checkedAdd(accessor.elementSize(), byteOffset)) <= view.byteLength); }; checkForall(accessors, [&](const Accessor &accessor) { if (accessor.bufferView.has_value())