From 4abb1f223cc1d06f18c2dd883172183e74aa07de Mon Sep 17 00:00:00 2001 From: BodgeMaster <> Date: Fri, 28 Oct 2022 04:08:06 +0200 Subject: [PATCH] lib/nbt: Lists can no longer be constructed from invalid sets of data The constructor `NBT::Tag::List::List(tiny_utf8::string, std::vector)` is now private and has a wrapper function `NBT::Tag::List::constructWithData(tiny_utf8::string, std::vector)` that performs a sanity check on the data it is given. --- src/lib/error.hpp | 2 ++ src/lib/nbt.cpp | 20 ++++++++++++++++++-- src/lib/nbt.hpp | 3 ++- src/test/nbt_tags.cpp | 28 +++++++++++++++------------- 4 files changed, 37 insertions(+), 16 deletions(-) diff --git a/src/lib/error.hpp b/src/lib/error.hpp index 4dd80a0..54b341f 100644 --- a/src/lib/error.hpp +++ b/src/lib/error.hpp @@ -95,6 +95,8 @@ namespace ErrorCodes { // never be performed (like deleting an end tag from an NBT compound) const uint8_t NOT_ALLOWED = 11; + const uint8_t MIXED_TYPES = 12; + const uint8_t UNIMPLEMENTED = 254; const uint8_t UNKNOWN = 255; diff --git a/src/lib/nbt.cpp b/src/lib/nbt.cpp index aa8cfc5..f60a3ca 100644 --- a/src/lib/nbt.cpp +++ b/src/lib/nbt.cpp @@ -861,6 +861,22 @@ namespace NBT { } } + ErrorOr List::constructWithData(tiny_utf8::string name, std::vector data) { + if (data.size() > 0xFFFFFFFF) { + return ErrorOr(true, ErrorCodes::OVERRUN, nullptr); + } + if (data.size() > 0) { + uint8_t dataType = data[0]->getTagType(); + for (uint32_t i=1; igetTagType() != dataType) { + return ErrorOr(true, ErrorCodes::MIXED_TYPES, nullptr); + } + } + } + + return ErrorOr(new List(name, data)); + } + List::~List() { for (uint64_t i=0; itags.size(); i++) { delete this->tags.at(i); @@ -1310,7 +1326,7 @@ namespace NBT { returnValue = ErrorOr>(true, nextListContents.errorCode); goto returnError; } - contents.push_back(new Tag::List("", nextListContents.value)); + contents.push_back(Tag::List::constructWithData("", nextListContents.value).value); *processedDataSize += *containedDataSize; } delete containedDataSize; @@ -1427,7 +1443,7 @@ namespace NBT { returnValue = ErrorOr>(true, listData.errorCode); goto returnNow; } - tags.push_back(new Tag::List(tagName.value, listData.value)); + tags.push_back(Tag::List::constructWithData(tagName.value, listData.value).value); *processedTagSize += (uint64_t) nameSize + 3; } if (data[currentPosition]==TagType::COMPOUND) { diff --git a/src/lib/nbt.hpp b/src/lib/nbt.hpp index 5e933dc..c0940e0 100644 --- a/src/lib/nbt.hpp +++ b/src/lib/nbt.hpp @@ -225,10 +225,10 @@ namespace NBT { private: std::vector tags; uint8_t containedType; + List(tiny_utf8::string name, std::vector data); public: List(); List(tiny_utf8::string name, uint8_t type); - List(tiny_utf8::string name, std::vector data); ~List() override; @@ -240,6 +240,7 @@ namespace NBT { ErrorOrVoid appendPointer(Generic* pointer); ErrorOrVoid deleteElement(uint64_t position); uint64_t length(); + static ErrorOr constructWithData(tiny_utf8::string name, std::vector data); }; class Compound: public Generic { diff --git a/src/test/nbt_tags.cpp b/src/test/nbt_tags.cpp index db2f005..1682c46 100644 --- a/src/test/nbt_tags.cpp +++ b/src/test/nbt_tags.cpp @@ -794,24 +794,26 @@ int main(){ ASSERT(resultError.isError && resultError.errorCode == ErrorCodes::INVALID_TYPE); //TODO: Check that constructing with a vector of mixed tags // results in a clearly defined failure mode (issue #60) - NBT::Tag::List list_2 = NBT::Tag::List("list_2", listDataVector); - ASSERT(list_2.getContainedType() == NBT::TagType::INT8); - ASSERT(list_1.length() == 4 && list_2.length() == 4); - ASSERT(!list_2.deleteElement(1).isError); - ASSERT(list_2.deleteElement(3).isError && list_2.deleteElement(3).errorCode == ErrorCodes::OUT_OF_RANGE); - ASSERT(!list_2.getElementPointer(2).isError && dynamic_cast(list_2.getElementPointer(2).value)->getValue() == 78); - ASSERT(list_2.getElementPointer(3).isError && list_2.getElementPointer(3).errorCode == ErrorCodes::OUT_OF_RANGE); - ASSERT(!list_2.setElementPointerAt(0, new NBT::Tag::Int8("set_entry", 3)).isError); - ErrorOrVoid resultRange = list_2.setElementPointerAt(3, new NBT::Tag::Int8("out_of_range_entry", 2)); + ErrorOr list_2_or_error = NBT::Tag::List::constructWithData("list_2", listDataVector); + ASSERT(!list_2_or_error.isError); + NBT::Tag::List* list_2 = list_2_or_error.value; + ASSERT(list_2->getContainedType() == NBT::TagType::INT8); + ASSERT(list_1.length() == 4 && list_2->length() == 4); + ASSERT(!list_2->deleteElement(1).isError); + ASSERT(list_2->deleteElement(3).isError && list_2->deleteElement(3).errorCode == ErrorCodes::OUT_OF_RANGE); + ASSERT(!list_2->getElementPointer(2).isError && dynamic_cast(list_2->getElementPointer(2).value)->getValue() == 78); + ASSERT(list_2->getElementPointer(3).isError && list_2->getElementPointer(3).errorCode == ErrorCodes::OUT_OF_RANGE); + ASSERT(!list_2->setElementPointerAt(0, new NBT::Tag::Int8("set_entry", 3)).isError); + ErrorOrVoid resultRange = list_2->setElementPointerAt(3, new NBT::Tag::Int8("out_of_range_entry", 2)); ASSERT(resultRange.isError); ASSERT(resultRange.errorCode == ErrorCodes::OUT_OF_RANGE); - ErrorOrVoid resultType = list_2.setElementPointerAt(0, new NBT::Tag::Int16()); + ErrorOrVoid resultType = list_2->setElementPointerAt(0, new NBT::Tag::Int16()); ASSERT(resultType.isError); ASSERT(resultType.errorCode == ErrorCodes::INVALID_TYPE); - ASSERT(list_2.length() == 3); + ASSERT(list_2->length() == 3); ASSERT(!list_1.serialize(&vector).isError); - ASSERT(!list_2.serialize(&vector).isError); + ASSERT(!list_2->serialize(&vector).isError); ASSERT(vector.size() == 35); ASSERT( vector.at( 0) == 9 && @@ -852,7 +854,7 @@ int main(){ ); vector.clear(); ASSERT(!list_1.serializeWithoutHeader(&vector).isError); - ASSERT(!list_2.serializeWithoutHeader(&vector).isError); + ASSERT(!list_2->serializeWithoutHeader(&vector).isError); ASSERT(vector.size() == 17); ASSERT( vector.at( 0) == 1 &&