From 269987085fbeef43af877f5eab8945a872ded650 Mon Sep 17 00:00:00 2001 From: Vinz Spring Date: Sun, 8 Jun 2025 13:46:39 +0200 Subject: [PATCH] Fixes CVE-2025-2750: out of bounds write by assigning to wrong array element count tracking (closes #6011) (#6225) description: - The current implementation has faulty reallocation logic when parsing a CSM file - Issue 1 - https://github.com/assimp/assimp/blob/4ad1d2aa3086517816716a50aa122342806736f9/code/AssetLib/CSM/CSMLoader.cpp#L205 - By assigning s->mNumPositionKeys = alloc*2 right before resizing the buffer, making s->mNumPositionKeys equivalent to the max number of aiVectorKey that can be stored in s->mPositionKeys - the code later attempts to get the next write location by doing: aiVectorKey* sub = s->mPositionKeys + s->mNumPositionKeys; - this points to the end of the array, not after the last element in the array - Issue 2 - https://github.com/assimp/assimp/blob/4ad1d2aa3086517816716a50aa122342806736f9/code/AssetLib/CSM/CSMLoader.cpp#L178-L184 - if the CSM file does not declare last frame data, then mPositionKeys will never be initialized fix: - we preserve s->mNumPositionKeys to still contain the actual number of aiVectorKeys and ensure that we will not write out of bounds - we initialize mPositionKeys with a default value and if we find last frame info, we just re-initialize it Co-authored-by: Vinz Spring Co-authored-by: Kim Kulling --- code/AssetLib/CSM/CSMLoader.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/code/AssetLib/CSM/CSMLoader.cpp b/code/AssetLib/CSM/CSMLoader.cpp index 497dc0aeb..b11861dcb 100644 --- a/code/AssetLib/CSM/CSMLoader.cpp +++ b/code/AssetLib/CSM/CSMLoader.cpp @@ -176,9 +176,17 @@ void CSMImporter::InternReadFile( const std::string& pFile, // If we know how many frames we'll read, we can preallocate some storage unsigned int alloc = 100; if (last != 0x00ffffff) { + // re-init if the file has last frame data alloc = last-first; alloc += alloc>>2u; // + 25% for (unsigned int i = 0; i < anim->mNumChannels; ++i) { + if (anim->mChannels[i]->mPositionKeys != nullptr) delete[] anim->mChannels[i]->mPositionKeys; + anim->mChannels[i]->mPositionKeys = new aiVectorKey[alloc]; + } + } else { + // default init + for (unsigned int i = 0; i < anim->mNumChannels; ++i) { + if (anim->mChannels[i]->mPositionKeys != nullptr) continue; anim->mChannels[i]->mPositionKeys = new aiVectorKey[alloc]; } } @@ -202,7 +210,7 @@ void CSMImporter::InternReadFile( const std::string& pFile, if (s->mNumPositionKeys == alloc) { // need to reallocate? aiVectorKey* old = s->mPositionKeys; - s->mPositionKeys = new aiVectorKey[s->mNumPositionKeys = alloc*2]; + s->mPositionKeys = new aiVectorKey[alloc*2]; ::memcpy(s->mPositionKeys,old,sizeof(aiVectorKey)*alloc); delete[] old; }