Fix heap-buffer-overflow in TelnetLayer due to stale offset in getOption/getOptionData (#2144) (#2148)

Author: alacrity-aya

Packet++/src/TelnetLayer.cpp

@@ -370,7 +370,10 @@ namespace pcpp
 			pos = getNextCommandField(pos, m_DataLen - offset);
 
 			if (pos && pos[1] == static_cast<int>(command))
+			{
+				offset = pos - m_Data;
 				return static_cast<TelnetOption>(getSubCommand(pos, getFieldLen(pos, m_DataLen - offset)));
+			}
 		}
 
 		PCPP_LOG_DEBUG("Can't find requested command");
@@ -417,8 +420,9 @@ namespace pcpp
 
 			if (pos && pos[1] == static_cast<int>(command))
 			{
-				size_t lenBuffer = getFieldLen(m_Data, m_DataLen);
-				uint8_t* posBuffer = getCommandData(m_Data, lenBuffer);
+				offset = pos - m_Data;
+				size_t lenBuffer = getFieldLen(pos, m_DataLen - offset);
+				uint8_t* posBuffer = getCommandData(pos, lenBuffer);
 
 				length = lenBuffer;
 				return posBuffer;

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

@@ -0,0 +1 @@
+00112233445566778899aabb08004500002e1234400040060000c0a80a01c0a80a0200173039000000000000000050187fff00000000fffffffa1700

Tests/Packet++Test/PacketExamples/telnetCommandAfterData.pcap

@@ -236,6 +236,7 @@ PTF_TEST_CASE(NtpCreationTests);
 
 // Implemented in TelnetTests.cpp
 PTF_TEST_CASE(TelnetCommandParsingTests);
+PTF_TEST_CASE(TelentCommandInvalidDataTests);
 PTF_TEST_CASE(TelnetDataParsingTests);
 
 // Implemented in IcmpV6Tests.cpp

Tests/Packet++Test/TestDefinition.h

@@ -255,6 +255,40 @@ PTF_TEST_CASE(TelnetCommandParsingTests)
 	}
 }
 
+PTF_TEST_CASE(TelentCommandInvalidDataTests)
+{
+	// Regression test for heap-buffer-overflow in getOption(TelnetCommand) / getOptionData(TelnetCommand, size_t&)
+	// when user data (escaped IAC) precedes a command. The stale offset bug caused reads past the buffer end.
+	// Payload: FF FF (escaped IAC = user data) + FF FA 17 00 (IAC SB SendLocation 0x00)
+	auto rawPacket1 = createPacketFromHexResource("PacketExamples/telnetCommandAfterData.dat");
+
+	pcpp::Packet telnetPacket(rawPacket1.get());
+	auto* telnetLayer = telnetPacket.getLayerOfType<pcpp::TelnetLayer>();
+
+	PTF_ASSERT_NOT_NULL(telnetLayer);
+
+	// Total commands: 1 (the subnegotiation)
+	PTF_ASSERT_EQUAL(telnetLayer->getTotalNumberOfCommands(), 1);
+
+	// getFirstCommand should find the subnegotiation
+	PTF_ASSERT_EQUAL(telnetLayer->getFirstCommand(), pcpp::TelnetLayer::TelnetCommand::Subnegotiation, enumclass);
+
+	// getOption(Subnegotiation) should return SendLocation without crashing
+	PTF_ASSERT_EQUAL(telnetLayer->getOption(pcpp::TelnetLayer::TelnetCommand::Subnegotiation),
+	                 pcpp::TelnetLayer::TelnetOption::SendLocation, enumclass);
+
+	// getOptionData(Subnegotiation) should return the subneg data without crashing
+	size_t length = 0;
+	auto* optData = telnetLayer->getOptionData(pcpp::TelnetLayer::TelnetCommand::Subnegotiation, length);
+	PTF_ASSERT_NOT_NULL(optData);
+	PTF_ASSERT_EQUAL(length, 1);
+	PTF_ASSERT_EQUAL(optData[0], 0x00);
+
+	// getOption for a command that doesn't exist should return NoOption without crashing
+	PTF_ASSERT_EQUAL(telnetLayer->getOption(pcpp::TelnetLayer::TelnetCommand::WillPerform),
+	                 pcpp::TelnetLayer::TelnetOption::TelnetOptionNoOption, enumclass);
+}
+
 PTF_TEST_CASE(TelnetDataParsingTests)
 {
 

Tests/Packet++Test/Tests/TelnetTests.cpp

@@ -325,6 +325,7 @@ int main(int argc, char* argv[])
 	PTF_RUN_TEST(NtpCreationTests, "ntp");
 
 	PTF_RUN_TEST(TelnetCommandParsingTests, "telnet");
+	PTF_RUN_TEST(TelentCommandInvalidDataTests, "telnet");
 	PTF_RUN_TEST(TelnetDataParsingTests, "telnet");
 
 	PTF_RUN_TEST(TpktLayerTest, "tpkt");