From 430fe98c5367538f415b2e438bda481a98e14371 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Je=C5=99=C3=A1bek?= Date: Thu, 22 Nov 2018 11:45:52 +0100 Subject: [PATCH] AssbinLoader: hardening, exception safety Fixes potential memory leaks and crashes on malformed input. --- code/AssbinLoader.cpp | 117 ++++++++++++++++--------------- include/assimp/MemoryIOWrapper.h | 3 +- 2 files changed, 61 insertions(+), 59 deletions(-) diff --git a/code/AssbinLoader.cpp b/code/AssbinLoader.cpp index ad91ce043..415d3576c 100644 --- a/code/AssbinLoader.cpp +++ b/code/AssbinLoader.cpp @@ -57,6 +57,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include #include #include +#include #ifdef ASSIMP_BUILD_NO_OWN_ZLIB # include @@ -103,7 +104,9 @@ bool AssbinImporter::CanRead( const std::string& pFile, IOSystem* pIOHandler, bo template T Read(IOStream * stream) { T t; - stream->Read( &t, sizeof(T), 1 ); + size_t res = stream->Read( &t, sizeof(T), 1 ); + if(res != 1) + throw DeadlyImportError("Unexpected EOF"); return t; } @@ -144,7 +147,8 @@ template <> aiString Read(IOStream * stream) { aiString s; stream->Read(&s.length,4,1); - stream->Read(s.data,s.length,1); + if(s.length) + stream->Read(s.data,s.length,1); s.data[s.length] = 0; return s; } @@ -207,46 +211,48 @@ void ReadBounds( IOStream * stream, T* /*p*/, unsigned int n ) { } // ----------------------------------------------------------------------------------- -void AssbinImporter::ReadBinaryNode( IOStream * stream, aiNode** node, aiNode* parent ) { - uint32_t chunkID = Read(stream); - (void)(chunkID); - ai_assert(chunkID == ASSBIN_CHUNK_AINODE); +void AssbinImporter::ReadBinaryNode( IOStream * stream, aiNode** onode, aiNode* parent ) { + if(Read(stream) != ASSBIN_CHUNK_AINODE) + throw DeadlyImportError("Magic chunk identifiers are wrong!"); /*uint32_t size =*/ Read(stream); - *node = new aiNode(); + std::unique_ptr node(new aiNode()); - (*node)->mName = Read(stream); - (*node)->mTransformation = Read(stream); - (*node)->mNumChildren = Read(stream); - (*node)->mNumMeshes = Read(stream); + node->mName = Read(stream); + node->mTransformation = Read(stream); + unsigned numChildren = Read(stream); + unsigned numMeshes = Read(stream); unsigned int nb_metadata = Read(stream); if(parent) { - (*node)->mParent = parent; + node->mParent = parent; } - if ((*node)->mNumMeshes) { - (*node)->mMeshes = new unsigned int[(*node)->mNumMeshes]; - for (unsigned int i = 0; i < (*node)->mNumMeshes; ++i) { - (*node)->mMeshes[i] = Read(stream); + if (numMeshes) + { + node->mMeshes = new unsigned int[numMeshes]; + for (unsigned int i = 0; i < numMeshes; ++i) { + node->mMeshes[i] = Read(stream); + node->mNumMeshes++; } } - if ((*node)->mNumChildren) { - (*node)->mChildren = new aiNode*[(*node)->mNumChildren]; - for (unsigned int i = 0; i < (*node)->mNumChildren; ++i) { - ReadBinaryNode( stream, &(*node)->mChildren[i], *node ); + if (numChildren) { + node->mChildren = new aiNode*[numChildren]; + for (unsigned int i = 0; i < numChildren; ++i) { + ReadBinaryNode( stream, &node->mChildren[i], node.get() ); } } + *onode = node.release(); if ( nb_metadata > 0 ) { - (*node)->mMetaData = aiMetadata::Alloc(nb_metadata); + node->mMetaData = aiMetadata::Alloc(nb_metadata); for (unsigned int i = 0; i < nb_metadata; ++i) { - (*node)->mMetaData->mKeys[i] = Read(stream); - (*node)->mMetaData->mValues[i].mType = (aiMetadataType) Read(stream); - void* data( nullptr ); + node->mMetaData->mKeys[i] = Read(stream); + node->mMetaData->mValues[i].mType = (aiMetadataType) Read(stream); + void* data = nullptr; - switch ((*node)->mMetaData->mValues[i].mType) { + switch (node->mMetaData->mValues[i].mType) { case AI_BOOL: data = new bool(Read(stream)); break; @@ -275,16 +281,15 @@ void AssbinImporter::ReadBinaryNode( IOStream * stream, aiNode** node, aiNode* p break; } - (*node)->mMetaData->mValues[i].mData = data; + node->mMetaData->mValues[i].mData = data; } } } // ----------------------------------------------------------------------------------- void AssbinImporter::ReadBinaryBone( IOStream * stream, aiBone* b ) { - uint32_t chunkID = Read(stream); - (void)(chunkID); - ai_assert(chunkID == ASSBIN_CHUNK_AIBONE); + if(Read(stream) != ASSBIN_CHUNK_AIBONE) + throw DeadlyImportError("Magic chunk identifiers are wrong!"); /*uint32_t size =*/ Read(stream); b->mName = Read(stream); @@ -306,12 +311,10 @@ void AssbinImporter::ReadBinaryBone( IOStream * stream, aiBone* b ) { static bool fitsIntoUI16(unsigned int mNumVertices) { return ( mNumVertices < (1u<<16) ); } - // ----------------------------------------------------------------------------------- void AssbinImporter::ReadBinaryMesh( IOStream * stream, aiMesh* mesh ) { - uint32_t chunkID = Read(stream); - (void)(chunkID); - ai_assert(chunkID == ASSBIN_CHUNK_AIMESH); + if(Read(stream) != ASSBIN_CHUNK_AIMESH) + throw DeadlyImportError("Magic chunk identifiers are wrong!"); /*uint32_t size =*/ Read(stream); mesh->mPrimitiveTypes = Read(stream); @@ -423,9 +426,8 @@ void AssbinImporter::ReadBinaryMesh( IOStream * stream, aiMesh* mesh ) { // ----------------------------------------------------------------------------------- void AssbinImporter::ReadBinaryMaterialProperty(IOStream * stream, aiMaterialProperty* prop) { - uint32_t chunkID = Read(stream); - (void)(chunkID); - ai_assert(chunkID == ASSBIN_CHUNK_AIMATERIALPROPERTY); + if(Read(stream) != ASSBIN_CHUNK_AIMATERIALPROPERTY) + throw DeadlyImportError("Magic chunk identifiers are wrong!"); /*uint32_t size =*/ Read(stream); prop->mKey = Read(stream); @@ -440,9 +442,8 @@ void AssbinImporter::ReadBinaryMaterialProperty(IOStream * stream, aiMaterialPro // ----------------------------------------------------------------------------------- void AssbinImporter::ReadBinaryMaterial(IOStream * stream, aiMaterial* mat) { - uint32_t chunkID = Read(stream); - (void)(chunkID); - ai_assert(chunkID == ASSBIN_CHUNK_AIMATERIAL); + if(Read(stream) != ASSBIN_CHUNK_AIMATERIAL) + throw DeadlyImportError("Magic chunk identifiers are wrong!"); /*uint32_t size =*/ Read(stream); mat->mNumAllocated = mat->mNumProperties = Read(stream); @@ -462,9 +463,8 @@ void AssbinImporter::ReadBinaryMaterial(IOStream * stream, aiMaterial* mat) { // ----------------------------------------------------------------------------------- void AssbinImporter::ReadBinaryNodeAnim(IOStream * stream, aiNodeAnim* nd) { - uint32_t chunkID = Read(stream); - (void)(chunkID); - ai_assert(chunkID == ASSBIN_CHUNK_AINODEANIM); + if(Read(stream) != ASSBIN_CHUNK_AINODEANIM) + throw DeadlyImportError("Magic chunk identifiers are wrong!"); /*uint32_t size =*/ Read(stream); nd->mNodeName = Read(stream); @@ -508,9 +508,8 @@ void AssbinImporter::ReadBinaryNodeAnim(IOStream * stream, aiNodeAnim* nd) { // ----------------------------------------------------------------------------------- void AssbinImporter::ReadBinaryAnim( IOStream * stream, aiAnimation* anim ) { - uint32_t chunkID = Read(stream); - (void)(chunkID); - ai_assert(chunkID == ASSBIN_CHUNK_AIANIMATION); + if(Read(stream) != ASSBIN_CHUNK_AIANIMATION) + throw DeadlyImportError("Magic chunk identifiers are wrong!"); /*uint32_t size =*/ Read(stream); anim->mName = Read (stream); @@ -529,9 +528,8 @@ void AssbinImporter::ReadBinaryAnim( IOStream * stream, aiAnimation* anim ) { // ----------------------------------------------------------------------------------- void AssbinImporter::ReadBinaryTexture(IOStream * stream, aiTexture* tex) { - uint32_t chunkID = Read(stream); - (void)(chunkID); - ai_assert(chunkID == ASSBIN_CHUNK_AITEXTURE); + if(Read(stream) != ASSBIN_CHUNK_AITEXTURE) + throw DeadlyImportError("Magic chunk identifiers are wrong!"); /*uint32_t size =*/ Read(stream); tex->mWidth = Read(stream); @@ -551,9 +549,8 @@ void AssbinImporter::ReadBinaryTexture(IOStream * stream, aiTexture* tex) { // ----------------------------------------------------------------------------------- void AssbinImporter::ReadBinaryLight( IOStream * stream, aiLight* l ) { - uint32_t chunkID = Read(stream); - (void)(chunkID); - ai_assert(chunkID == ASSBIN_CHUNK_AILIGHT); + if(Read(stream) != ASSBIN_CHUNK_AILIGHT) + throw DeadlyImportError("Magic chunk identifiers are wrong!"); /*uint32_t size =*/ Read(stream); l->mName = Read(stream); @@ -577,9 +574,8 @@ void AssbinImporter::ReadBinaryLight( IOStream * stream, aiLight* l ) { // ----------------------------------------------------------------------------------- void AssbinImporter::ReadBinaryCamera( IOStream * stream, aiCamera* cam ) { - uint32_t chunkID = Read(stream); - (void)(chunkID); - ai_assert(chunkID == ASSBIN_CHUNK_AICAMERA); + if(Read(stream) != ASSBIN_CHUNK_AICAMERA) + throw DeadlyImportError("Magic chunk identifiers are wrong!"); /*uint32_t size =*/ Read(stream); cam->mName = Read(stream); @@ -594,9 +590,8 @@ void AssbinImporter::ReadBinaryCamera( IOStream * stream, aiCamera* cam ) { // ----------------------------------------------------------------------------------- void AssbinImporter::ReadBinaryScene( IOStream * stream, aiScene* scene ) { - uint32_t chunkID = Read(stream); - (void)(chunkID); - ai_assert(chunkID == ASSBIN_CHUNK_AISCENE); + if(Read(stream) != ASSBIN_CHUNK_AISCENE) + throw DeadlyImportError("Magic chunk identifiers are wrong!"); /*uint32_t size =*/ Read(stream); scene->mFlags = Read(stream); @@ -614,6 +609,7 @@ void AssbinImporter::ReadBinaryScene( IOStream * stream, aiScene* scene ) { // Read all meshes if (scene->mNumMeshes) { scene->mMeshes = new aiMesh*[scene->mNumMeshes]; + memset(scene->mMeshes, 0, scene->mNumMeshes*sizeof(aiMesh*)); for (unsigned int i = 0; i < scene->mNumMeshes;++i) { scene->mMeshes[i] = new aiMesh(); ReadBinaryMesh( stream,scene->mMeshes[i]); @@ -623,6 +619,7 @@ void AssbinImporter::ReadBinaryScene( IOStream * stream, aiScene* scene ) { // Read materials if (scene->mNumMaterials) { scene->mMaterials = new aiMaterial*[scene->mNumMaterials]; + memset(scene->mMaterials, 0, scene->mNumMaterials*sizeof(aiMaterial*)); for (unsigned int i = 0; i< scene->mNumMaterials; ++i) { scene->mMaterials[i] = new aiMaterial(); ReadBinaryMaterial(stream,scene->mMaterials[i]); @@ -632,6 +629,7 @@ void AssbinImporter::ReadBinaryScene( IOStream * stream, aiScene* scene ) { // Read all animations if (scene->mNumAnimations) { scene->mAnimations = new aiAnimation*[scene->mNumAnimations]; + memset(scene->mAnimations, 0, scene->mNumAnimations*sizeof(aiAnimation*)); for (unsigned int i = 0; i < scene->mNumAnimations;++i) { scene->mAnimations[i] = new aiAnimation(); ReadBinaryAnim(stream,scene->mAnimations[i]); @@ -641,6 +639,7 @@ void AssbinImporter::ReadBinaryScene( IOStream * stream, aiScene* scene ) { // Read all textures if (scene->mNumTextures) { scene->mTextures = new aiTexture*[scene->mNumTextures]; + memset(scene->mTextures, 0, scene->mNumTextures*sizeof(aiTexture*)); for (unsigned int i = 0; i < scene->mNumTextures;++i) { scene->mTextures[i] = new aiTexture(); ReadBinaryTexture(stream,scene->mTextures[i]); @@ -650,6 +649,7 @@ void AssbinImporter::ReadBinaryScene( IOStream * stream, aiScene* scene ) { // Read lights if (scene->mNumLights) { scene->mLights = new aiLight*[scene->mNumLights]; + memset(scene->mLights, 0, scene->mNumLights*sizeof(aiLight*)); for (unsigned int i = 0; i < scene->mNumLights;++i) { scene->mLights[i] = new aiLight(); ReadBinaryLight(stream,scene->mLights[i]); @@ -659,6 +659,7 @@ void AssbinImporter::ReadBinaryScene( IOStream * stream, aiScene* scene ) { // Read cameras if (scene->mNumCameras) { scene->mCameras = new aiCamera*[scene->mNumCameras]; + memset(scene->mCameras, 0, scene->mNumCameras*sizeof(aiCamera*)); for (unsigned int i = 0; i < scene->mNumCameras;++i) { scene->mCameras[i] = new aiCamera(); ReadBinaryCamera(stream,scene->mCameras[i]); @@ -675,7 +676,7 @@ void AssbinImporter::InternReadFile( const std::string& pFile, aiScene* pScene, } // signature - stream->Seek( 44, aiOrigin_CUR ); + stream->Seek( 44, aiOrigin_CUR ); unsigned int versionMajor = Read(stream); unsigned int versionMinor = Read(stream); diff --git a/include/assimp/MemoryIOWrapper.h b/include/assimp/MemoryIOWrapper.h index bfcfff9c2..54d89aa64 100644 --- a/include/assimp/MemoryIOWrapper.h +++ b/include/assimp/MemoryIOWrapper.h @@ -80,7 +80,8 @@ public: // ------------------------------------------------------------------- // Read from stream size_t Read(void* pvBuffer, size_t pSize, size_t pCount) { - const size_t cnt = std::min(pCount,(length-pos)/pSize),ofs = pSize*cnt; + ai_assert(pvBuffer && pSize); + const size_t cnt = std::min(pCount,(length-pos)/pSize), ofs = pSize*cnt; memcpy(pvBuffer,buffer+pos,ofs); pos += ofs;