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 <vinzs@amazon.de>
Co-authored-by: Kim Kulling <kimkulling@users.noreply.github.com>
This commit is contained in:
Vinz Spring
2025-06-08 13:46:39 +02:00
committed by GitHub
co-authored by Vinz Spring Kim Kulling
parent 5be336779d
commit 269987085f
+9 -1
View File
@@ -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;
}