Fixed a container-overflow vulnerability in Assimp::ColladaLoader::BuildMeshesForNode (#6575)
The root cause was an unchecked access to the `newMats` vector using `matIdx`. When a material name (`meshMaterial`) is not found in `mMaterialIndexByName`, `matIdx` defaults to 0. If `newMats` is empty (which happens if the material library is empty or failed to load), accessing `newMats[0]` results in a container-overflow. The fix involves adding a bounds check `matIdx < newMats.size()` to the condition guarding the access to `newMats`. Additionally, based on maintainer feedback, I added a warning log `ASSIMP_LOG_WARN` when the index is out of bounds, to inform the user about the missing material definition or broken reference. I verified the fix using the provided reproduction command. The container-overflow is no longer triggered. I also built and ran the unit tests. Since `bin/unit` was missing, I manually built the unit tests using `cmake.real` and `ninja` in `/src/assimp/build_tests` with `ASSIMP_BUILD_TESTS=ON`, `ASSIMP_BUILD_ZLIB=ON`, and `ASSIMP_WARNINGS_AS_ERRORS=OFF` (to bypass a gtest compilation warning), and updated `run_tests.sh` to point to the built binary. All 584 tests passed. Fixes: https://issues.oss-fuzz.com/issues/483102958 Signed-off-by: Bill Wendling <morbo@google.com> Co-authored-by: CodeMender <codemender-patching@google.com> Co-authored-by: Kim Kulling <kimkulling@users.noreply.github.com>
This commit is contained in:
@@ -507,16 +507,20 @@ void ColladaLoader::BuildMeshesForNode(const ColladaParser &pParser, const Node
|
||||
}
|
||||
|
||||
if (table && !table->mMap.empty()) {
|
||||
std::pair<Collada::Effect *, aiMaterial *> &mat = newMats[matIdx];
|
||||
if (matIdx < newMats.size()) {
|
||||
std::pair<Collada::Effect *, aiMaterial *> &mat = newMats[matIdx];
|
||||
|
||||
// Iterate through all texture channels assigned to the effect and
|
||||
// check whether we have mapping information for it.
|
||||
ApplyVertexToEffectSemanticMapping(mat.first->mTexDiffuse, *table);
|
||||
ApplyVertexToEffectSemanticMapping(mat.first->mTexAmbient, *table);
|
||||
ApplyVertexToEffectSemanticMapping(mat.first->mTexSpecular, *table);
|
||||
ApplyVertexToEffectSemanticMapping(mat.first->mTexEmissive, *table);
|
||||
ApplyVertexToEffectSemanticMapping(mat.first->mTexTransparent, *table);
|
||||
ApplyVertexToEffectSemanticMapping(mat.first->mTexBump, *table);
|
||||
// Iterate through all texture channels assigned to the effect and
|
||||
// check whether we have mapping information for it.
|
||||
ApplyVertexToEffectSemanticMapping(mat.first->mTexDiffuse, *table);
|
||||
ApplyVertexToEffectSemanticMapping(mat.first->mTexAmbient, *table);
|
||||
ApplyVertexToEffectSemanticMapping(mat.first->mTexSpecular, *table);
|
||||
ApplyVertexToEffectSemanticMapping(mat.first->mTexEmissive, *table);
|
||||
ApplyVertexToEffectSemanticMapping(mat.first->mTexTransparent, *table);
|
||||
ApplyVertexToEffectSemanticMapping(mat.first->mTexBump, *table);
|
||||
} else {
|
||||
ASSIMP_LOG_WARN("Collada: Ignoring material mapping for mesh \"", mid.mMeshOrController, "\". Material index ", matIdx, " is out of bounds (newMats.size()=", newMats.size(), ").");
|
||||
}
|
||||
}
|
||||
|
||||
// built lookup index of the Mesh-Submesh-Material combination
|
||||
|
||||
Reference in New Issue
Block a user