Added missing checks on array lengths in BinaryParser3.1.0

4 files changed, 26 insertions, 16 deletions

diff --git a/src/GCodes/GCodeBuffer/BinaryParser.cpp b/src/GCodes/GCodeBuffer/BinaryParser.cpp
index cfd33999..8106f7b8 100644
--- a/src/GCodes/GCodeBuffer/BinaryParser.cpp
+++ b/src/GCodes/GCodeBuffer/BinaryParser.cpp
@@ -452,6 +452,7 @@ void BinaryParser::GetDriverIdArray(DriverId arr[], size_t& length) THROWS(GCode
 	case DataType::IntArray:
 	case DataType::UIntArray:
 	case DataType::DriverIdArray:
+		CheckArrayLength(length);
 		for (int i = 0; i < seenParameter->intValue; i++)
 		{
 			arr[i].SetFromBinary(reinterpret_cast<const uint32_t*>(seenParameterValue)[i]);
@@ -548,6 +549,7 @@ template<typename T> void BinaryParser::GetArray(T arr[], size_t& length, bool d
 		lastIndex = 0;
 		break;
 	case DataType::IntArray:
+		CheckArrayLength(length);
 		for (int i = 0; i < seenParameter->intValue; i++)
 		{
 			arr[i] = reinterpret_cast<const int32_t*>(seenParameterValue)[i];
@@ -556,6 +558,7 @@ template<typename T> void BinaryParser::GetArray(T arr[], size_t& length, bool d
 		break;
 	case DataType::DriverIdArray:
 	case DataType::UIntArray:
+		CheckArrayLength(length);
 		for (int i = 0; i < seenParameter->intValue; i++)
 		{
 			arr[i] = reinterpret_cast<const uint32_t*>(seenParameterValue)[i];
@@ -563,6 +566,7 @@ template<typename T> void BinaryParser::GetArray(T arr[], size_t& length, bool d
 		lastIndex = seenParameter->intValue - 1;
 		break;
 	case DataType::FloatArray:
+		CheckArrayLength(length);
 		for (int i = 0; i < seenParameter->intValue; i++)
 		{
 			arr[i] = reinterpret_cast<const float*>(seenParameterValue)[i];
@@ -587,6 +591,14 @@ template<typename T> void BinaryParser::GetArray(T arr[], size_t& length, bool d
 	}
 }
 
+void BinaryParser::CheckArrayLength(size_t maxLength) THROWS(GCodeException)
+{
+	if ((unsigned int)seenParameter->intValue > maxLength)
+	{
+		throw ConstructParseException("array too long, max length = %u", (uint32_t)maxLength);
+	}
+}
+
 void BinaryParser::WriteParameters(const StringRef& s, bool quoteStrings) const noexcept
 {
 	if (bufferLength != 0)
diff --git a/src/GCodes/GCodeBuffer/BinaryParser.h b/src/GCodes/GCodeBuffer/BinaryParser.h
index 9514adba..8a367511 100644
--- a/src/GCodes/GCodeBuffer/BinaryParser.h
+++ b/src/GCodes/GCodeBuffer/BinaryParser.h
@@ -62,6 +62,7 @@ public:
 private:
 	GCodeBuffer& gb;
 
+	void CheckArrayLength(size_t maxLength) THROWS(GCodeException);
 	GCodeException ConstructParseException(const char *str) const noexcept;
 	GCodeException ConstructParseException(const char *str, const char *param) const noexcept;
 	GCodeException ConstructParseException(const char *str, uint32_t param) const noexcept;
diff --git a/src/GCodes/GCodeBuffer/StringParser.cpp b/src/GCodes/GCodeBuffer/StringParser.cpp
index 38d4ddb0..26bd531a 100644
--- a/src/GCodes/GCodeBuffer/StringParser.cpp
+++ b/src/GCodes/GCodeBuffer/StringParser.cpp
@@ -950,10 +950,7 @@ void StringParser::GetFloatArray(float arr[], size_t& returnedLength, bool doPad
 	size_t length = 0;
 	for (;;)
 	{
-		if (length >= returnedLength)		// array limit has been set in here
-		{
-			throw ConstructParseException("array too long, max length = %u", (uint32_t)returnedLength);
-		}
+		CheckArrayLength(length, returnedLength);
 		arr[length++] = ReadFloatValue();
 		if (gb.buffer[readPointer] != LIST_SEPARATOR)
 		{
@@ -989,10 +986,7 @@ void StringParser::GetIntArray(int32_t arr[], size_t& returnedLength, bool doPad
 	size_t length = 0;
 	for (;;)
 	{
-		if (length >= returnedLength) // Array limit has been set in here
-		{
-			throw ConstructParseException("array too long, max length = %u", (uint32_t)returnedLength);
-		}
+		CheckArrayLength(length, returnedLength);
 		arr[length] = ReadIValue();
 		length++;
 		if (gb.buffer[readPointer] != LIST_SEPARATOR)
@@ -1028,10 +1022,7 @@ void StringParser::GetUnsignedArray(uint32_t arr[], size_t& returnedLength, bool
 	size_t length = 0;
 	for (;;)
 	{
-		if (length >= returnedLength) // Array limit has been set in here
-		{
-			throw ConstructParseException("array too long, max length = %u", (uint32_t)returnedLength);
-		}
+		CheckArrayLength(length, returnedLength);
 		arr[length] = ReadUIValue();
 		length++;
 		if (gb.buffer[readPointer] != LIST_SEPARATOR)
@@ -1068,10 +1059,7 @@ void StringParser::GetDriverIdArray(DriverId arr[], size_t& returnedLength) THRO
 	size_t length = 0;
 	for (;;)
 	{
-		if (length >= returnedLength) // Array limit has been set in here
-		{
-			throw ConstructParseException("array too long, max length = %u", (uint32_t)returnedLength);
-		}
+		CheckArrayLength(length, returnedLength);
 		arr[length] = ReadDriverIdValue();
 		length++;
 		if (gb.buffer[readPointer] != LIST_SEPARATOR)
@@ -1085,6 +1073,14 @@ void StringParser::GetDriverIdArray(DriverId arr[], size_t& returnedLength) THRO
 	readPointer = -1;
 }
 
+void StringParser::CheckArrayLength(size_t actualLength, size_t maxLength) THROWS(GCodeException)
+{
+	if (actualLength >= maxLength)
+	{
+		throw ConstructParseException("array too long, max length = %u", (uint32_t)maxLength);
+	}
+}
+
 // Get and copy a quoted string returning true if successful
 void StringParser::GetQuotedString(const StringRef& str, bool allowEmpty) THROWS(GCodeException)
 {
diff --git a/src/GCodes/GCodeBuffer/StringParser.h b/src/GCodes/GCodeBuffer/StringParser.h
index c1c30743..60259cc4 100644
--- a/src/GCodes/GCodeBuffer/StringParser.h
+++ b/src/GCodes/GCodeBuffer/StringParser.h
@@ -100,6 +100,7 @@ private:
 	uint32_t ReadUIValue() THROWS(GCodeException);
 	int32_t ReadIValue() THROWS(GCodeException);
 	DriverId ReadDriverIdValue() THROWS(GCodeException);
+	void CheckArrayLength(size_t actualLength, size_t maxLength) THROWS(GCodeException);
 
 	void CheckForMixedSpacesAndTabs() noexcept;
 	bool ProcessConditionalGCode(const StringRef& reply, BlockType skippedBlockType, bool doingFile) THROWS(GCodeException);