diff --git a/changes.txt b/changes.txt index d68c94f..59d63c5 100644 --- a/changes.txt +++ b/changes.txt @@ -400,6 +400,10 @@ Changes in 1.9 (not yet released) -------------------------- Changes in 1.8.6 +- Fix CB3DMeshFileLoader::readString. Prevent adding a character beyond file-end. Thanks @sfan5 for report and patch. + Original patch (commit 103ab16 to Minetest): https://github.com/minetest/irrlicht/commit/103ab16679a42cb1bfa4cc4e6316195ec2d139b6 +- CB3DMeshFileLoader: add some bounds checks. Thanks @sfan5 for report and patch. + Original patch (commit 64688f4 to Minetest): https://github.com/minetest/irrlicht/commit/64688f449099246ec27eb013f58d72a0abb1c6e6 - TGA loader: Fix number overflow causing crashes. Thanks @sfan5 for fuzzing test. - TGA loader: Fix several buffer overflows. Thanks @erlehmann for report and @sfan5 for fuzzing test: https://github.com/minetest/irrlicht/issues/236 - COBJMeshFilerLoder: prevent buffer overruns from loading files passing negative indices. Thanks @sfan5 fore report and patch. diff --git a/source/Irrlicht/CB3DMeshFileLoader.cpp b/source/Irrlicht/CB3DMeshFileLoader.cpp index 3e4a0c1..97c0133 100644 --- a/source/Irrlicht/CB3DMeshFileLoader.cpp +++ b/source/Irrlicht/CB3DMeshFileLoader.cpp @@ -255,7 +255,7 @@ bool CB3DMeshFileLoader::readChunkMESH(CSkinnedMesh::SJoint *inJoint) os::Printer::log(logStr.c_str(), ELL_DEBUG); #endif - s32 brushID; + s32 brushID=-1; B3DFile->read(&brushID, sizeof(brushID)); #ifdef __BIG_ENDIAN__ brushID = os::Byteswap::byteswap(brushID); @@ -283,11 +283,15 @@ bool CB3DMeshFileLoader::readChunkMESH(CSkinnedMesh::SJoint *inJoint) { scene::SSkinMeshBuffer *meshBuffer = AnimatedMesh->addMeshBuffer(); - if (brushID!=-1) + if ( brushID >= 0 && (u32)brushID < Materials.size() ) { loadTextures(Materials[brushID]); meshBuffer->Material=Materials[brushID].Material; } + else if (brushID != -1) // -1 is OK + { + os::Printer::log("Illegal brush ID found", B3DFile->getFileName(), ELL_WARNING); + } if(readChunkTRIS(meshBuffer,AnimatedMesh->getMeshBuffers().size()-1, VerticesStart)==false) return false; @@ -364,7 +368,8 @@ bool CB3DMeshFileLoader::readChunkVRTS(CSkinnedMesh::SJoint *inJoint) tex_coord_set_size = os::Byteswap::byteswap(tex_coord_set_size); #endif - if (tex_coord_sets >= max_tex_coords || tex_coord_set_size >= 4) // Something is wrong + if (tex_coord_sets < 0 || tex_coord_set_size < 0 || + tex_coord_sets >= max_tex_coords || tex_coord_set_size >= 4) // Something is wrong { os::Printer::log("tex_coord_sets or tex_coord_set_size too big", B3DFile->getFileName(), ELL_ERROR); return false; @@ -460,7 +465,7 @@ bool CB3DMeshFileLoader::readChunkTRIS(scene::SSkinMeshBuffer *meshBuffer, u32 m bool showVertexWarning=false; - s32 triangle_brush_id; // Note: Irrlicht can't have different brushes for each triangle (using a workaround) + s32 triangle_brush_id=-1; // Note: Irrlicht can't have different brushes for each triangle (using a workaround) B3DFile->read(&triangle_brush_id, sizeof(triangle_brush_id)); #ifdef __BIG_ENDIAN__ triangle_brush_id = os::Byteswap::byteswap(triangle_brush_id); @@ -468,14 +473,20 @@ bool CB3DMeshFileLoader::readChunkTRIS(scene::SSkinMeshBuffer *meshBuffer, u32 m SB3dMaterial *B3dMaterial; - if (triangle_brush_id != -1) + if (triangle_brush_id >= 0 && (u32)triangle_brush_id < Materials.size()) { loadTextures(Materials[triangle_brush_id]); B3dMaterial = &Materials[triangle_brush_id]; meshBuffer->Material = B3dMaterial->Material; } else + { B3dMaterial = 0; + if (triangle_brush_id < -1) // -1 is OK + { + os::Printer::log("Illegal material index for triangle brush found", B3DFile->getFileName(), ELL_WARNING); + } + } const s32 memoryNeeded = B3dStack.getLast().length / sizeof(s32); meshBuffer->Indices.reallocate(memoryNeeded + meshBuffer->Indices.size() + 1); @@ -594,7 +605,11 @@ bool CB3DMeshFileLoader::readChunkBONE(CSkinnedMesh::SJoint *inJoint) #endif globalVertexID += VerticesStart; - if (AnimatedVertices_VertexID[globalVertexID]==-1) + if (globalVertexID >= AnimatedVertices_VertexID.size()) + { + os::Printer::log("Illegal vertex index found", B3DFile->getFileName(), ELL_WARNING); + } + else if (AnimatedVertices_VertexID[globalVertexID]==-1) { os::Printer::log("B3dMeshLoader: Weight has bad vertex id (no link to meshbuffer index found)"); } @@ -1081,10 +1096,9 @@ void CB3DMeshFileLoader::loadTextures(SB3dMaterial& material) const void CB3DMeshFileLoader::readString(core::stringc& newstring) { newstring=""; - while (B3DFile->getPos() <= B3DFile->getSize()) + c8 character=0; + while (B3DFile->read(&character, sizeof(character)) > 0) // until eof { - c8 character; - B3DFile->read(&character, sizeof(character)); if (character==0) return; newstring.append(character); diff --git a/source/Irrlicht/COBJMeshFileLoader.cpp b/source/Irrlicht/COBJMeshFileLoader.cpp index 02d6c72..398bd9a 100644 --- a/source/Irrlicht/COBJMeshFileLoader.cpp +++ b/source/Irrlicht/COBJMeshFileLoader.cpp @@ -74,13 +74,14 @@ IAnimatedMesh* COBJMeshFileLoader::createMesh(io::IReadFile* file) const io::path fullName = file->getFileName(); const io::path relPath = FileSystem->getFileDir(fullName)+"/"; - c8* buf = new c8[filesize]; + c8* buf = new c8[filesize+1]; // plus null-terminator (some string functions used in parsing) filesize = file->read((void*)buf, filesize); if ( filesize <= 0 ) { delete[] buf; return 0; } + buf[filesize] = 0; if ( getMeshTextureLoader() ) getMeshTextureLoader()->setMeshFile(file); diff --git a/source/Irrlicht/SB3DStructs.h b/source/Irrlicht/SB3DStructs.h index 8f88a85..dfec8f8 100644 --- a/source/Irrlicht/SB3DStructs.h +++ b/source/Irrlicht/SB3DStructs.h @@ -12,6 +12,7 @@ #define SB3DSTRUCTS_H #include "SMaterial.h" +#include "irrMath.h" namespace irr { namespace scene { @@ -27,6 +28,7 @@ struct SB3dChunk SB3dChunk(const SB3dChunkHeader& header, long sp) : length(header.size+8), startposition(sp) { + length = core::max_(length, 8); name[0]=header.name[0]; name[1]=header.name[1]; name[2]=header.name[2];