Fix heap-buffer-overflow in ModbusLayer on truncated packets (#2159)

Signed-off-by: Sai Asish Y <say.apm35@gmail.com>

Author: SAY-5

Packet++/header/ModbusLayer.h

@@ -101,6 +101,15 @@ namespace pcpp
 			return port == 502;
 		}
 
+		/// A static method that checks whether the data is large enough to hold a MODBUS header
+		/// @param[in] data A pointer to the raw data
+		/// @param[in] dataSize The size of the data in bytes
+		/// @return True if the data is large enough to be parsed as a MODBUS layer, false otherwise
+		static bool isDataValid(const uint8_t* data, size_t dataSize)
+		{
+			return data != nullptr && dataSize >= sizeof(modbus_header);
+		}
+
 		/// @return MODBUS message type
 		uint16_t getTransactionId() const;
 

Packet++/src/TcpLayer.cpp

@@ -482,7 +482,7 @@ namespace pcpp
 		{
 			constructNextLayer<GtpV2Layer>(payload, payloadLen, getAttachedPacket());
 		}
-		else if (ModbusLayer::isModbusPort(portDst))
+		else if (ModbusLayer::isModbusPort(portDst) && ModbusLayer::isDataValid(payload, payloadLen))
 		{
 			constructNextLayer<ModbusLayer>(payload, payloadLen, getAttachedPacket());
 		}

Tests/Packet++Test/PacketExamples/ModbusTruncated.dat

@@ -0,0 +1 @@
+000c29af7ffe109add4e060d080045000038c6e64000400658880a0101ea0a0a0555c8d301f6e072efbac405c33a8018ffffd5d900000101080a37c3fee900ba2a0f00110000

Tests/Packet++Test/TestDefinition.h

@@ -329,6 +329,7 @@ PTF_TEST_CASE(CiscoHdlcLayerEditTest);
 // Implemented in ModbusTests.cpp
 PTF_TEST_CASE(ModbusLayerCreationTest);
 PTF_TEST_CASE(ModbusLayerParsingTest);
+PTF_TEST_CASE(ModbusLayerTruncatedTest);
 
 // Implemented in X509Tests.cpp
 PTF_TEST_CASE(X509ParsingTest);

Tests/Packet++Test/Tests/ModbusTests.cpp

@@ -51,3 +51,14 @@ PTF_TEST_CASE(ModbusLayerParsingTest)
 	PTF_ASSERT_EQUAL(modbusLayer->toString(),
 	                 "Modbus Layer, Transaction ID: 17, Protocol ID: 0, Length: 6, Unit ID: 255, Function Code: 4");
 }  // ModbusLayerParsingTest
+
+PTF_TEST_CASE(ModbusLayerTruncatedTest)
+{
+	// A packet to port 502 whose payload is shorter than a full modbus_header (8 bytes).
+	// It must not be parsed as a Modbus layer, otherwise the header getters read out of bounds.
+	auto rawPacket1 = createPacketFromHexResource("PacketExamples/ModbusTruncated.dat");
+
+	pcpp::Packet packet(rawPacket1.get());
+	PTF_ASSERT_FALSE(packet.isPacketOfType(pcpp::Modbus));
+	PTF_ASSERT_NULL(packet.getLayerOfType<pcpp::ModbusLayer>());
+}  // ModbusLayerTruncatedTest

Tests/Packet++Test/main.cpp

@@ -401,6 +401,7 @@ int main(int argc, char* argv[])
 
 	PTF_RUN_TEST(ModbusLayerCreationTest, "modbus");
 	PTF_RUN_TEST(ModbusLayerParsingTest, "modbus");
+	PTF_RUN_TEST(ModbusLayerTruncatedTest, "modbus");
 
 	PTF_RUN_TEST(X509ParsingTest, "x509");
 	PTF_RUN_TEST(X509VariantsParsingTest, "x509");