lib/varint: Fix error handling for out of bounds access

src/lib/cli.cpp

@@ -202,7 +202,6 @@ namespace CLI {
     }
 
     ArgumentsParser::~ArgumentsParser() {
-        //TODO: check that this actually runs
         for (auto const& [shortName, flag]: this->flagsByShortName) {
             delete flag;
         }

src/lib/varint.hpp

@@ -26,7 +26,7 @@ namespace VarInt {
     // up to 5 bytes, least significant byte first, most significant bit
     // indicates whether the next byte is still part of the number
     ErrorOr<int32_t> fromVar32(std::vector<uint8_t> data, uint64_t initialPosition=0, uint8_t* processedBytes=nullptr) {
-        if (initialPosition > data.size()) {
+        if (initialPosition >= data.size()) {
             return ErrorOr<int32_t>(true, ErrorCodes::OUT_OF_RANGE);
         }
 
@@ -40,7 +40,7 @@ namespace VarInt {
             bits += 7;
             currentPosition++;
             // check after increasing so we don't need to check outside the loop
-            if (currentPosition > data.size()) {
+            if (currentPosition >= data.size()) {
                 return ErrorOr<int32_t>(true, ErrorCodes::OVERRUN);
             }
 
@@ -57,7 +57,7 @@ namespace VarInt {
     // up to 10 bytes, least significant byte first, most significant bit
     // indicates whether the next byte is still part of the number
     ErrorOr<int64_t> fromVar64(std::vector<uint8_t> data, uint64_t initialPosition=0, uint8_t* processedBytes=nullptr) {
-        if (initialPosition > data.size()) {
+        if (initialPosition >= data.size()) {
             return ErrorOr<int64_t>(true, ErrorCodes::OUT_OF_RANGE);
         }
 
@@ -71,7 +71,7 @@ namespace VarInt {
             bits += 7;
             currentPosition++;
             // check after increasing so we don't need to check outside the loop
-            if (currentPosition > data.size()) {
+            if (currentPosition >= data.size()) {
                 return ErrorOr<int64_t>(true, ErrorCodes::OVERRUN);
             }
         }

src/test/varint.cpp

@@ -42,6 +42,30 @@ int main() {
     // 1000 0000 0100 0000 0010 0000 0001 0000 0000 1000 0000 0100 0000 0010 0000 0001 = -9205322385119247871 -> 1000 0001 1000 0100 1001 0000 1100 0000 1000 0000 1000 0010 1000 1000 1010 0000 1000 0000 0000 0001
     // 1111 1111 1111 1111 1111 1111 1111 1111 1111 1111 1111 1111 1111 1111 1111 1111 = -1 (unsigned int64 max) -> 1111 1111 1111 1111 1111 1111 1111 1111 1111 1111 1111 1111 1111 1111 1111 1111 1111 1111 0000 0001
 
+    {
+        std::vector<uint8_t> data = std::vector<uint8_t>();
+        uint8_t processedBytes;
+        ErrorOr<int32_t> result = VarInt::fromVar32(data, 0, &processedBytes);
+        ASSERT(result.isError);
+        ASSERT(result.errorCode == ErrorCodes::OUT_OF_RANGE);
+    }
+
+    {
+        std::vector<uint8_t> data = { 0x84 };
+        uint8_t processedBytes;
+        ErrorOr<int32_t> result = VarInt::fromVar32(data, 0, &processedBytes);
+        ASSERT(result.isError);
+        ASSERT(result.errorCode == ErrorCodes::OVERRUN);
+    }
+
+    {
+        std::vector<uint8_t> data = { 0xff, 0xff, 0xff, 0xff, 0xff, 0x01 };
+        uint8_t processedBytes;
+        ErrorOr<int32_t> result = VarInt::fromVar32(data, 0, &processedBytes);
+        ASSERT(result.isError);
+        ASSERT(result.errorCode == ErrorCodes::OVERFLOW);
+    }
+
     uint8_t zeroProcessedBytes = 0;
     std::vector<uint8_t> zeroData;
     zeroData.push_back(0);
@@ -90,6 +114,30 @@ int main() {
 
     std::cout << "Passed fromVar32 test." << std::endl;
 
+    {
+        std::vector<uint8_t> data = std::vector<uint8_t>();
+        uint8_t processedBytes;
+        ErrorOr<int64_t> result = VarInt::fromVar64(data, 0, &processedBytes);
+        ASSERT(result.isError);
+        ASSERT(result.errorCode == ErrorCodes::OUT_OF_RANGE);
+    }
+
+    {
+        std::vector<uint8_t> data = { 0x84 };
+        uint8_t processedBytes;
+        ErrorOr<int64_t> result = VarInt::fromVar64(data, 0, &processedBytes);
+        ASSERT(result.isError);
+        ASSERT(result.errorCode == ErrorCodes::OVERRUN);
+    }
+
+    {
+        std::vector<uint8_t> data = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x01 };
+        uint8_t processedBytes;
+        ErrorOr<int64_t> result = VarInt::fromVar64(data, 0, &processedBytes);
+        ASSERT(result.isError);
+        ASSERT(result.errorCode == ErrorCodes::OVERFLOW);
+    }
+
     uint8_t zero64ProcessedBytes = 0;
     std::vector<uint8_t> zero64Data;
     zero64Data.push_back(0);