From 390eeb111b9404f1cba5dd4eabc77592bdb78763 Mon Sep 17 00:00:00 2001 From: vabr-g Date: Tue, 2 Mar 2021 11:55:19 +0100 Subject: [PATCH] Fix stack overflow in parsing URDF files in Bullet When reading response from the Bullet server after a "load URDF" command, the client code allocated and zeroed buffers for the data chunks with a "+1" to account for the terminating zero. This was when the buffer pointer was interpreted as a `char *`, so it meant "+ 1 byte". When reading those buffers, however, the associated pointer was a `void *`, so reading `sizeof(void*)` (8 on my machine) bytes at a time. Therefore it was reading up to 7 bytes past the allocated (and zeroed) memory. The change fixes that by changing the "+ 1" to "+ sizeof(void*)". At one place it also extends the zeroing to the final buffer position (missing "+ 1" in the original). --- Extras/Serialize/BulletFileLoader/bFile.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Extras/Serialize/BulletFileLoader/bFile.cpp b/Extras/Serialize/BulletFileLoader/bFile.cpp index da9f65f20..5c8f54fe3 100644 --- a/Extras/Serialize/BulletFileLoader/bFile.cpp +++ b/Extras/Serialize/BulletFileLoader/bFile.cpp @@ -643,8 +643,8 @@ char *bFile::readStruct(char *head, bChunkInd &dataChunk) if ((strcmp(oldType, "btShortIntIndexData") == 0)) { int allocLen = 2; - char *dataAlloc = new char[(dataChunk.nr * allocLen) + 1]; - memset(dataAlloc, 0, (dataChunk.nr * allocLen) + 1); + char *dataAlloc = new char[(dataChunk.nr * allocLen) + sizeof(void*)]; + memset(dataAlloc, 0, (dataChunk.nr * allocLen) + sizeof(void*)); short *dest = (short *)dataAlloc; const short *src = (short *)head; for (int i = 0; i < dataChunk.nr; i++) @@ -682,8 +682,8 @@ char *bFile::readStruct(char *head, bChunkInd &dataChunk) // numBlocks * length int allocLen = (curLen); - char *dataAlloc = new char[(dataChunk.nr * allocLen) + 1]; - memset(dataAlloc, 0, (dataChunk.nr * allocLen)); + char *dataAlloc = new char[(dataChunk.nr * allocLen) + sizeof(void*)]; + memset(dataAlloc, 0, (dataChunk.nr * allocLen) + sizeof(void*)); // track allocated addDataBlock(dataAlloc); @@ -719,8 +719,8 @@ char *bFile::readStruct(char *head, bChunkInd &dataChunk) #endif // } - char *dataAlloc = new char[(dataChunk.len) + 1]; - memset(dataAlloc, 0, dataChunk.len + 1); + char *dataAlloc = new char[(dataChunk.len) + sizeof(void*)]; + memset(dataAlloc, 0, dataChunk.len + sizeof(void*)); // track allocated addDataBlock(dataAlloc);