spnda / fastgltf

A modern C++17 glTF 2.0 library focused on speed, correctness, and usability
https://fastgltf.readthedocs.io/v0.8.x/
MIT License
275 stars 42 forks source link

Bug in fg::Parser::generateMeshIndices(): accessor.count assignment uses invalid positionAccessor reference #55

Closed droettger closed 6 months ago

droettger commented 6 months ago

Error in fg::Parser::generateMeshIndices() when assigning accessor.count from dead positionAccessor.count data. Results in "bad allcoation" error when trying to allocate the given huge accessor.count data later. Repro: MSVS 2022 Debug 64 build. Load the file https://github.com/KhronosGroup/glTF-Sample-Models/blob/main/2.0/TriangleWithoutIndices/glTF/TriangleWithoutIndices.gltf with fastgltf::Options::GenerateMeshIndices enabled. I used these settings

  constexpr auto gltfOptions = fastgltf::Options::None 
    | fastgltf::Options::DontRequireValidAssetMember
    | fastgltf::Options::LoadGLBBuffers
    | fastgltf::Options::LoadExternalBuffers
    | fastgltf::Options::LoadExternalImages
    | fastgltf::Options::GenerateMeshIndices;

BUG: The fg::Parser::generateMeshIndices() function assignment accessor.count = positionAccessor.count; in line 895 is using stale data because the auto& accessor = asset.accessors.emplace_back(); changed the accessor vector and invalidates the previously read positionAccessor reference. In debug mode, all positionAccessor data becomes 0xdddddddd, which is a huge count and results in an allocation error when used later.

droettger commented 6 months ago

Wow, that was quick. Thanks a lot.