From 25d7806f6d0a0d826dfc29bd6d18b26f1e53241c Mon Sep 17 00:00:00 2001 From: BodgeMaster <> Date: Mon, 17 Oct 2022 16:01:57 +0200 Subject: [PATCH] lib/nbt: Fix memory leak in case of parsing error --- src/lib/nbt.cpp | 53 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 35 insertions(+), 18 deletions(-) diff --git a/src/lib/nbt.cpp b/src/lib/nbt.cpp index 5264ecf..aa8cfc5 100644 --- a/src/lib/nbt.cpp +++ b/src/lib/nbt.cpp @@ -1176,6 +1176,7 @@ namespace NBT { // FIXME: memory leak when returning errors ErrorOr> deserializeRawListContents(uint8_t data[], uint64_t dataSize, uint64_t initialPosition, uint64_t* processedDataSize) { std::vector contents; + ErrorOr> returnValue; // get contained data length by reading it manually because // the function that does it normally can't deal with // headerless tags @@ -1183,6 +1184,8 @@ namespace NBT { // add one byte to position to skip the type byte ErrorOr elementCount = Helper::readInt32(data, dataSize, initialPosition+1); if (elementCount.isError) { + // this is before the creation of any pointers so we can just return + // without using the returnError label at the end of this function return ErrorOr>(true, elementCount.errorCode); } @@ -1202,14 +1205,10 @@ namespace NBT { for (int32_t i=0; i nextInt = Helper::readInt8(data, dataSize, initialPosition+*processedDataSize); if (nextInt.isError) { - return ErrorOr>(true, nextInt.errorCode); + returnValue = ErrorOr>(true, nextInt.errorCode); + goto returnError; } contents.push_back(new Tag::Int8("", nextInt.value)); - // The below code would produce a warning on GCC and Clang - // about the computed value not being used. While this does - // apply inside this function, it is ultimately not true - // as the pointer is used both inside and outside of the - // function. *processedDataSize += 1; } break; @@ -1218,7 +1217,8 @@ namespace NBT { for (int32_t i=0; i nextInt = Helper::readInt16(data, dataSize, initialPosition+*processedDataSize); if (nextInt.isError) { - return ErrorOr>(true, nextInt.errorCode); + returnValue = ErrorOr>(true, nextInt.errorCode); + goto returnError; } contents.push_back(new Tag::Int16("", nextInt.value)); *processedDataSize += 2; @@ -1229,7 +1229,8 @@ namespace NBT { for (int32_t i=0; i nextInt = Helper::readInt32(data, dataSize, initialPosition+*processedDataSize); if (nextInt.isError) { - return ErrorOr>(true, nextInt.errorCode); + returnValue = ErrorOr>(true, nextInt.errorCode); + goto returnError; } contents.push_back(new Tag::Int32("", nextInt.value)); *processedDataSize += 4; @@ -1240,7 +1241,8 @@ namespace NBT { for (int32_t i=0; i nextFloat = Helper::readFloat(data, dataSize, initialPosition+*processedDataSize); if (nextFloat.isError) { - return ErrorOr>(true, nextFloat.errorCode); + returnValue = ErrorOr>(true, nextFloat.errorCode); + goto returnError; } contents.push_back(new Tag::Float("", nextFloat.value)); *processedDataSize += 4; @@ -1251,7 +1253,8 @@ namespace NBT { for (int32_t i=0; i nextInt = Helper::readInt64(data, dataSize, initialPosition+*processedDataSize); if (nextInt.isError) { - return ErrorOr>(true, nextInt.errorCode); + returnValue = ErrorOr>(true, nextInt.errorCode); + goto returnError; } contents.push_back(new Tag::Int64("", nextInt.value)); *processedDataSize += 8; @@ -1262,7 +1265,8 @@ namespace NBT { for (int32_t i=0; i nextDouble = Helper::readDouble(data, dataSize, initialPosition+*processedDataSize); if (nextDouble.isError) { - return ErrorOr>(true, nextDouble.errorCode); + returnValue = ErrorOr>(true, nextDouble.errorCode); + goto returnError; } contents.push_back(new Tag::Double("", nextDouble.value)); *processedDataSize += 8; @@ -1273,7 +1277,8 @@ namespace NBT { for (int32_t i=0; i> nextArray = Helper::readInt8Array(data, dataSize, initialPosition+*processedDataSize); if (nextArray.isError) { - return ErrorOr>(true, nextArray.errorCode); + returnValue = ErrorOr>(true, nextArray.errorCode); + goto returnError; } contents.push_back(new Tag::Int8Array("", nextArray.value)); *processedDataSize += (uint64_t) nextArray.value.size(); @@ -1284,7 +1289,8 @@ namespace NBT { for (int32_t i=0; i nextString = Helper::readString(data, dataSize, initialPosition+*processedDataSize); if (nextString.isError) { - return ErrorOr>(true, nextString.errorCode); + returnValue = ErrorOr>(true, nextString.errorCode); + goto returnError; } contents.push_back(new Tag::String("", nextString.value)); // this cannot be an error because it just got read @@ -1301,7 +1307,8 @@ namespace NBT { ErrorOr> nextListContents = deserializeRawListContents(data, dataSize, initialPosition+*processedDataSize, containedDataSize); if (nextListContents.isError) { delete containedDataSize; - return ErrorOr>(true, nextListContents.errorCode); + returnValue = ErrorOr>(true, nextListContents.errorCode); + goto returnError; } contents.push_back(new Tag::List("", nextListContents.value)); *processedDataSize += *containedDataSize; @@ -1316,7 +1323,8 @@ namespace NBT { ErrorOr> nextCompoundData = deserialize(data, dataSize, initialPosition+*processedDataSize, containedDataSize); if (nextCompoundData.isError) { delete containedDataSize; - return ErrorOr>(true, nextCompoundData.errorCode); + returnValue = ErrorOr>(true, nextCompoundData.errorCode); + goto returnError; } contents.push_back(new Tag::Compound("", nextCompoundData.value)); *processedDataSize += *containedDataSize; @@ -1328,7 +1336,8 @@ namespace NBT { for (int32_t i=0; i> nextArray = Helper::readInt32Array(data, dataSize, initialPosition+*processedDataSize); if (nextArray.isError) { - return ErrorOr>(true, nextArray.errorCode); + returnValue = ErrorOr>(true, nextArray.errorCode); + goto returnError; } contents.push_back(new Tag::Int32Array("", nextArray.value)); *processedDataSize += (uint64_t) nextArray.value.size() * 4; @@ -1339,7 +1348,8 @@ namespace NBT { for (int32_t i=0; i> nextArray = Helper::readInt64Array(data, dataSize, initialPosition+*processedDataSize); if (nextArray.isError) { - return ErrorOr>(true, nextArray.errorCode); + returnValue = ErrorOr>(true, nextArray.errorCode); + goto returnError; } contents.push_back(new Tag::Int64Array("", nextArray.value)); *processedDataSize += (uint64_t) nextArray.value.size() * 8; @@ -1347,9 +1357,16 @@ namespace NBT { break; } default: - return ErrorOr>(true, ErrorCodes::INVALID_TYPE); + returnValue = ErrorOr>(true, ErrorCodes::INVALID_TYPE); + goto returnError; } return ErrorOr>(contents); + + returnError: + for (uint64_t i=0; i