From 9190cad80d865aa79723f98deb1dccf59afe1ca6 Mon Sep 17 00:00:00 2001 From: BodgeMaster <> Date: Mon, 15 Aug 2022 08:50:07 +0200 Subject: [PATCH] lib/nbt: finish implementation of validateRawNBTData() and fix a critical macro-induced bug I did a `#define return` and then tried to `if () return;` everywhere... --- src/lib/nbt.cpp | 149 ++++++++++++++++++++++++++++++++++++++++++++---- src/lib/nbt.hpp | 2 +- 2 files changed, 140 insertions(+), 11 deletions(-) diff --git a/src/lib/nbt.cpp b/src/lib/nbt.cpp index 671c46f..91d2b3b 100644 --- a/src/lib/nbt.cpp +++ b/src/lib/nbt.cpp @@ -569,6 +569,116 @@ namespace NBT { } } + bool validateRawList(uint8_t data[], uint64_t dataSize, uint64_t initialPosition, uint64_t* processedDataSize) { + ErrorOr elementCount = helper::containedDataLength(data, dataSize, initialPosition); + if (elementCount.isError) { + return false; + } + // there is no way this is an error bc it gets checked while trying + // to get the element count + int16_t nameSize = helper::readInt16(data, dataSize, initialPosition+1).value; + // type byte + two name size bytes = 3 + uint8_t contentType = data[initialPosition + nameSize + 3]; + // type byte + two name size bytes + contained type byte + 4 length bytes = 8 + *processedDataSize = 8; + switch (contentType) { + case TagType::END: + // everything except content has been touched at this point + // and a list of end tags has no content + return true; + case TagType::INT8: { + *processedDataSize += (uint64_t) elementCount.value; + return initialPosition + *processedDataSize < dataSize; + } + case TagType::INT16: { + *processedDataSize += (uint64_t) elementCount.value * 2; + return initialPosition + *processedDataSize < dataSize; + } + case TagType::INT32: + case TagType::FLOAT: { + *processedDataSize += (uint64_t) elementCount.value * 4; + return initialPosition + *processedDataSize < dataSize; + } + case TagType::INT64: + case TagType::DOUBLE: { + *processedDataSize += (uint64_t) elementCount.value * 8; + return initialPosition + *processedDataSize < dataSize; + } + case TagType::INT8_ARRAY: { + for (int32_t i=0; i> nextArray = helper::readInt8Array(data, dataSize, initialPosition+*processedDataSize); + if (nextArray.isError) { + return false; + } + *processedDataSize += (uint64_t) nextArray.value.size(); + } + return true; + } + case TagType::STRING: { + for (int32_t i=0; i nextString = helper::readString(data, dataSize, initialPosition+*processedDataSize); + if (nextString.isError) { + return false; + } + // this cannot be an error because it just got checked + int16_t nextStringSize = helper::readInt16(data, dataSize, initialPosition+*processedDataSize).value; + *processedDataSize += (uint64_t) nextStringSize + 2; + } + return true; + } + case TagType::LIST: { + uint64_t* containedDataSize = new uint64_t; + for (int32_t i=0; i> nextArray = helper::readInt32Array(data, dataSize, initialPosition+*processedDataSize); + if (nextArray.isError) { + return false; + } + *processedDataSize += (uint64_t) nextArray.value.size() * 4; + } + return true; + } + case TagType::INT64_ARRAY: { + for (int32_t i=0; i> nextArray = helper::readInt64Array(data, dataSize, initialPosition+*processedDataSize); + if (nextArray.isError) { + return false; + } + *processedDataSize += (uint64_t) nextArray.value.size() * 8; + } + return true; + } + default: + return false; + } + } + bool validateRawNBTData(uint8_t data[], uint64_t dataSize, uint64_t initialPosition, uint64_t* processedDataSize){ if (initialPosition >= dataSize) { // Yes, this *could* return an instance of ErrorOr with @@ -594,21 +704,27 @@ namespace NBT { // // - BodgeMaster } + uint64_t currentPosition = initialPosition; #define return if (processedDataSize!=nullptr) *processedDataSize = currentPosition-initialPosition; return while (currentPosition nextTagSize = helper::totalTagSize(data, dataSize, currentPosition); if (nextTagSize.isError) { if (nextTagSize.errorCode == ErrorCodes::NOT_YET_KNOWN) { + // attempt parsing the name + ErrorOr tagName = helper::readString(data, dataSize, currentPosition+1); + if (tagName.isError) { + return false; + } + uint64_t* processedTagSize = new uint64_t; *processedTagSize = 0; - // attempt parsing the name - ErrorOr tagName = helper::readString(data, dataSize, currentPosition+1); - if (tagName.isError) return false; - if (data[currentPosition]==TagType::LIST) { - //TODO: handle list + if (!validateRawList(data, dataSize, currentPosition, processedTagSize)) { + delete processedTagSize; + return false; + } } if (data[currentPosition]==TagType::COMPOUND) { // seek to the start of the compound's contents @@ -617,19 +733,28 @@ namespace NBT { // checked while trying to parse the string above int16_t nameSize = helper::readInt16(data, dataSize, currentPosition+1).value; - if (!validateRawNBTData(data, dataSize, currentPosition + (uint64_t) nameSize + 1, processedTagSize)) return false; + if (!validateRawNBTData(data, dataSize, currentPosition + (uint64_t) nameSize + 1, processedTagSize)) { + delete processedTagSize; + return false; + } *processedTagSize += (uint64_t) nameSize + 1; } currentPosition += *processedTagSize; + + delete processedTagSize; continue; } return false; } - if (currentPosition + nextTagSize.value > dataSize) return false; + if (currentPosition + nextTagSize.value > dataSize) { + return false; + } // recursion abort condition - if (data[currentPosition]==TagType::END) return true; + if (data[currentPosition]==TagType::END) { + return true; + } // nameSize cannot be an error here bc it got checked in // nextTagSize() already @@ -643,7 +768,9 @@ namespace NBT { // if (currentPosition + nextTagSize.value > dataSize) return false; // It might, however, turn out to be a useful check in the future. ErrorOr name = helper::readString(data, dataSize, currentPosition+1); - if (name.isError) return false; + if (name.isError) { + return false; + } switch (data[0]) { case TagType::INT8: @@ -666,7 +793,9 @@ namespace NBT { // // type byte + two name size bytes = 3 ErrorOr content = helper::readString(data, dataSize, currentPosition+nameSize+3); - if (content.isError) return false; + if (content.isError) { + return false; + } break; } case TagType::INT32_ARRAY: diff --git a/src/lib/nbt.hpp b/src/lib/nbt.hpp index 81a735e..b473112 100644 --- a/src/lib/nbt.hpp +++ b/src/lib/nbt.hpp @@ -113,5 +113,5 @@ namespace NBT { bool validate(uint8_t data[]); }; - bool validateRawNBTData(uint8_t data[], int length, uint64_t initialPosition, uint64_t* processedDataSize=nullptr); + bool validateRawNBTData(uint8_t data[], uint64_t dataSize, uint64_t initialPosition, uint64_t* processedDataSize=nullptr); }