Add boundary checks to prevent memory safety issues in BgpLayer::getHeaderLen (#1954)

Author: danasana

Packet++/header/BgpLayer.h

@@ -113,6 +113,10 @@ namespace pcpp
 		}
 
 		void setBgpFields(size_t messageLen = 0);
+
+		bool extendLayer(int offsetInLayer, size_t numOfBytesToExtend) override;
+
+		bool shortenLayer(int offsetInLayer, size_t numOfBytesToShorten) override;
 	};
 
 	/// @class BgpOpenMessageLayer

Packet++/src/BgpLayer.cpp

@@ -2,6 +2,7 @@
 
 #include "Logger.h"
 #include "BgpLayer.h"
+#include "Packet.h"
 #include "EndianPortable.h"
 #include "GeneralUtils.h"
 
@@ -117,6 +118,56 @@ namespace pcpp
 		}
 	}
 
+	bool BgpLayer::extendLayer(int offsetInLayer, size_t numOfBytesToExtend)
+	{
+		if (m_Packet != nullptr)
+		{
+			int rawPacketLen = m_Packet->getRawPacket()->getRawDataLen();
+			const uint8_t* rawPacketPtr = m_Packet->getRawPacket()->getRawData();
+
+			if (m_Data - rawPacketPtr + static_cast<ptrdiff_t>(offsetInLayer) > static_cast<ptrdiff_t>(rawPacketLen))
+			{
+				PCPP_LOG_ERROR("Requested offset is larger than total packet length");
+				return false;
+			}
+
+			if (m_NextLayer != nullptr && static_cast<ptrdiff_t>(offsetInLayer) > m_NextLayer->getData() - m_Data)
+			{
+				PCPP_LOG_ERROR("Requested offset exceeds current layer's boundary");
+				return false;
+			}
+		}
+
+		return Layer::extendLayer(offsetInLayer, numOfBytesToExtend);
+	}
+
+	bool BgpLayer::shortenLayer(int offsetInLayer, size_t numOfBytesToShorten)
+	{
+		if (m_Packet != nullptr)
+		{
+			int rawPacketLen = m_Packet->getRawPacket()->getRawDataLen();
+			const uint8_t* rawPacketPtr = m_Packet->getRawPacket()->getRawData();
+
+			if (m_Data - rawPacketPtr + static_cast<ptrdiff_t>(offsetInLayer) +
+			        static_cast<ptrdiff_t>(numOfBytesToShorten) >
+			    static_cast<ptrdiff_t>(rawPacketLen))
+			{
+				PCPP_LOG_ERROR("Requested number of bytes to shorten is larger than total packet length");
+				return false;
+			}
+
+			if (m_NextLayer != nullptr &&
+			    static_cast<ptrdiff_t>(offsetInLayer) + static_cast<ptrdiff_t>(numOfBytesToShorten) >
+			        m_NextLayer->getData() - m_Data)
+			{
+				PCPP_LOG_ERROR("Requested number of bytes to shorten exceeds current layer's boundary");
+				return false;
+			}
+		}
+
+		return Layer::shortenLayer(offsetInLayer, numOfBytesToShorten);
+	}
+
 	// ~~~~~~~~~~~~~~~~~~~~
 	// BgpOpenMessageLayer
 	// ~~~~~~~~~~~~~~~~~~~~
@@ -261,29 +312,32 @@ namespace pcpp
 		uint8_t newOptionalParamsData[1500];
 		size_t newOptionalParamsDataLen = optionalParamsToByteArray(optionalParameters, newOptionalParamsData, 1500);
 		size_t curOptionalParamsDataLen = getOptionalParametersLength();
+		int offsetInLayer = sizeof(bgp_open_message);
 
 		if (newOptionalParamsDataLen > curOptionalParamsDataLen)
 		{
-			bool res = extendLayer(sizeof(bgp_open_message), newOptionalParamsDataLen - curOptionalParamsDataLen);
-			if (!res)
+			size_t numOfBytesToExtend = newOptionalParamsDataLen - curOptionalParamsDataLen;
+
+			if (!extendLayer(offsetInLayer, numOfBytesToExtend))
 			{
 				PCPP_LOG_ERROR("Couldn't extend BGP open layer to include the additional optional parameters");
-				return res;
+				return false;
 			}
 		}
 		else if (newOptionalParamsDataLen < curOptionalParamsDataLen)
 		{
-			bool res = shortenLayer(sizeof(bgp_open_message), curOptionalParamsDataLen - newOptionalParamsDataLen);
-			if (!res)
+			size_t numOfBytesToShorten = curOptionalParamsDataLen - newOptionalParamsDataLen;
+
+			if (!shortenLayer(offsetInLayer, numOfBytesToShorten))
 			{
 				PCPP_LOG_ERROR("Couldn't shorten BGP open layer to set the right size of the optional parameters data");
-				return res;
+				return false;
 			}
 		}
 
 		if (newOptionalParamsDataLen > 0)
 		{
-			memcpy(m_Data + sizeof(bgp_open_message), newOptionalParamsData, newOptionalParamsDataLen);
+			memcpy(m_Data + offsetInLayer, newOptionalParamsData, newOptionalParamsDataLen);
 		}
 
 		getOpenMsgHeader()->optionalParameterLength = (uint8_t)newOptionalParamsDataLen;
@@ -565,32 +619,32 @@ namespace pcpp
 		uint8_t newWithdrawnRoutesData[1500];
 		size_t newWithdrawnRoutesDataLen = prefixAndIPDataToByteArray(withdrawnRoutes, newWithdrawnRoutesData, 1500);
 		size_t curWithdrawnRoutesDataLen = getWithdrawnRoutesLength();
+		int offsetInLayer = sizeof(bgp_common_header) + sizeof(uint16_t);
 
 		if (newWithdrawnRoutesDataLen > curWithdrawnRoutesDataLen)
 		{
-			bool res = extendLayer(sizeof(bgp_common_header) + sizeof(uint16_t),
-			                       newWithdrawnRoutesDataLen - curWithdrawnRoutesDataLen);
-			if (!res)
+			size_t numOfBytesToExtend = newWithdrawnRoutesDataLen - curWithdrawnRoutesDataLen;
+
+			if (!extendLayer(offsetInLayer, numOfBytesToExtend))
 			{
 				PCPP_LOG_ERROR("Couldn't extend BGP update layer to include the additional withdrawn routes");
-				return res;
+				return false;
 			}
 		}
 		else if (newWithdrawnRoutesDataLen < curWithdrawnRoutesDataLen)
 		{
-			bool res = shortenLayer(sizeof(bgp_common_header) + sizeof(uint16_t),
-			                        curWithdrawnRoutesDataLen - newWithdrawnRoutesDataLen);
-			if (!res)
+			size_t numOfBytesToShorten = curWithdrawnRoutesDataLen - newWithdrawnRoutesDataLen;
+
+			if (!shortenLayer(offsetInLayer, numOfBytesToShorten))
 			{
 				PCPP_LOG_ERROR("Couldn't shorten BGP update layer to set the right size of the withdrawn routes data");
-				return res;
+				return false;
 			}
 		}
 
 		if (newWithdrawnRoutesDataLen > 0)
 		{
-			memcpy(m_Data + sizeof(bgp_common_header) + sizeof(uint16_t), newWithdrawnRoutesData,
-			       newWithdrawnRoutesDataLen);
+			memcpy(m_Data + offsetInLayer, newWithdrawnRoutesData, newWithdrawnRoutesDataLen);
 		}
 
 		getBasicHeader()->length =
@@ -642,32 +696,32 @@ namespace pcpp
 		size_t newPathAttributesDataLen = pathAttributesToByteArray(pathAttributes, newPathAttributesData, 1500);
 		size_t curPathAttributesDataLen = getPathAttributesLength();
 		size_t curWithdrawnRoutesDataLen = getWithdrawnRoutesLength();
+		int offsetInLayer = sizeof(bgp_common_header) + 2 * sizeof(uint16_t) + curWithdrawnRoutesDataLen;
 
 		if (newPathAttributesDataLen > curPathAttributesDataLen)
 		{
-			bool res = extendLayer(sizeof(bgp_common_header) + 2 * sizeof(uint16_t) + curWithdrawnRoutesDataLen,
-			                       newPathAttributesDataLen - curPathAttributesDataLen);
-			if (!res)
+			size_t numOfBytesToExtend = newPathAttributesDataLen - curPathAttributesDataLen;
+
+			if (!extendLayer(offsetInLayer, numOfBytesToExtend))
 			{
 				PCPP_LOG_ERROR("Couldn't extend BGP update layer to include the additional path attributes");
-				return res;
+				return false;
 			}
 		}
 		else if (newPathAttributesDataLen < curPathAttributesDataLen)
 		{
-			bool res = shortenLayer(sizeof(bgp_common_header) + 2 * sizeof(uint16_t) + curWithdrawnRoutesDataLen,
-			                        curPathAttributesDataLen - newPathAttributesDataLen);
-			if (!res)
+			size_t numOfBytesToShorten = curPathAttributesDataLen - newPathAttributesDataLen;
+
+			if (!shortenLayer(offsetInLayer, numOfBytesToShorten))
 			{
 				PCPP_LOG_ERROR("Couldn't shorten BGP update layer to set the right size of the path attributes data");
-				return res;
+				return false;
 			}
 		}
 
 		if (newPathAttributesDataLen > 0)
 		{
-			memcpy(m_Data + sizeof(bgp_common_header) + 2 * sizeof(uint16_t) + curWithdrawnRoutesDataLen,
-			       newPathAttributesData, newPathAttributesDataLen);
+			memcpy(m_Data + offsetInLayer, newPathAttributesData, newPathAttributesDataLen);
 		}
 
 		getBasicHeader()->length =
@@ -741,35 +795,33 @@ namespace pcpp
 		size_t curNlriDataLen = getNetworkLayerReachabilityInfoLength();
 		size_t curPathAttributesDataLen = getPathAttributesLength();
 		size_t curWithdrawnRoutesDataLen = getWithdrawnRoutesLength();
+		int offsetInLayer =
+		    sizeof(bgp_common_header) + 2 * sizeof(uint16_t) + curWithdrawnRoutesDataLen + curPathAttributesDataLen;
 
 		if (newNlriDataLen > curNlriDataLen)
 		{
-			bool res = extendLayer(sizeof(bgp_common_header) + 2 * sizeof(uint16_t) + curWithdrawnRoutesDataLen +
-			                           curPathAttributesDataLen,
-			                       newNlriDataLen - curNlriDataLen);
-			if (!res)
+			size_t numOfBytesToExtend = newNlriDataLen - curNlriDataLen;
+
+			if (!extendLayer(offsetInLayer, numOfBytesToExtend))
 			{
 				PCPP_LOG_ERROR("Couldn't extend BGP update layer to include the additional NLRI data");
-				return res;
+				return false;
 			}
 		}
 		else if (newNlriDataLen < curNlriDataLen)
 		{
-			bool res = shortenLayer(sizeof(bgp_common_header) + 2 * sizeof(uint16_t) + curWithdrawnRoutesDataLen +
-			                            curPathAttributesDataLen,
-			                        curNlriDataLen - newNlriDataLen);
-			if (!res)
+			size_t numOfBytesToShorten = curNlriDataLen - newNlriDataLen;
+
+			if (!shortenLayer(offsetInLayer, numOfBytesToShorten))
 			{
 				PCPP_LOG_ERROR("Couldn't shorten BGP update layer to set the right size of the NLRI data");
-				return res;
+				return false;
 			}
 		}
 
 		if (newNlriDataLen > 0)
 		{
-			memcpy(m_Data + sizeof(bgp_common_header) + 2 * sizeof(uint16_t) + curWithdrawnRoutesDataLen +
-			           curPathAttributesDataLen,
-			       newNlriData, newNlriDataLen);
+			memcpy(m_Data + offsetInLayer, newNlriData, newNlriDataLen);
 		}
 
 		getBasicHeader()->length = htobe16(be16toh(getBasicHeader()->length) + newNlriDataLen - curNlriDataLen);
@@ -866,30 +918,33 @@ namespace pcpp
 		}
 
 		size_t curNotificationDataLen = getNotificationDataLen();
+		int offsetInLayer = sizeof(bgp_notification_message);
 
 		if (newNotificationDataLen > curNotificationDataLen)
 		{
-			bool res = extendLayer(sizeof(bgp_notification_message), newNotificationDataLen - curNotificationDataLen);
-			if (!res)
+			size_t numOfBytesToExtend = newNotificationDataLen - curNotificationDataLen;
+
+			if (!extendLayer(offsetInLayer, numOfBytesToExtend))
 			{
 				PCPP_LOG_ERROR("Couldn't extend BGP notification layer to include the additional notification data");
-				return res;
+				return false;
 			}
 		}
 		else if (newNotificationDataLen < curNotificationDataLen)
 		{
-			bool res = shortenLayer(sizeof(bgp_notification_message), curNotificationDataLen - newNotificationDataLen);
-			if (!res)
+			size_t numOfBytesToShorten = curNotificationDataLen - newNotificationDataLen;
+
+			if (!shortenLayer(offsetInLayer, numOfBytesToShorten))
 			{
 				PCPP_LOG_ERROR(
 				    "Couldn't shorten BGP notification layer to set the right size of the notification data");
-				return res;
+				return false;
 			}
 		}
 
 		if (newNotificationDataLen > 0)
 		{
-			memcpy(m_Data + sizeof(bgp_notification_message), newNotificationData, newNotificationDataLen);
+			memcpy(m_Data + offsetInLayer, newNotificationData, newNotificationDataLen);
 		}
 
 		getNotificationMsgHeader()->length = htobe16(sizeof(bgp_notification_message) + newNotificationDataLen);

Packet++/src/Layer.cpp

@@ -63,12 +63,11 @@ namespace pcpp
 
 		if (m_Packet == nullptr)
 		{
-			if ((size_t)offsetInLayer > m_DataLen)
+			if (static_cast<size_t>(offsetInLayer) > m_DataLen)
 			{
 				PCPP_LOG_ERROR("Requested offset is larger than data length");
 				return false;
 			}
-
 			uint8_t* newData = new uint8_t[m_DataLen + numOfBytesToExtend];
 			memcpy(newData, m_Data, offsetInLayer);
 			memcpy(newData + offsetInLayer + numOfBytesToExtend, m_Data + offsetInLayer, m_DataLen - offsetInLayer);
@@ -89,14 +88,19 @@ namespace pcpp
 			return false;
 		}
 
+		if (static_cast<size_t>(offsetInLayer) + numOfBytesToShorten > m_DataLen)
+		{
+			PCPP_LOG_ERROR("Requested number of bytes to shorten is larger than data length");
+			return false;
+		}
+
 		if (m_Packet == nullptr)
 		{
-			if ((size_t)offsetInLayer >= m_DataLen)
+			if (static_cast<size_t>(offsetInLayer) >= m_DataLen)
 			{
 				PCPP_LOG_ERROR("Requested offset is larger than data length");
 				return false;
 			}
-
 			uint8_t* newData = new uint8_t[m_DataLen - numOfBytesToShorten];
 			memcpy(newData, m_Data, offsetInLayer);
 			memcpy(newData + offsetInLayer, m_Data + offsetInLayer + numOfBytesToShorten,

Packet++/src/Packet.cpp

@@ -605,6 +605,12 @@ namespace pcpp
 		bool passedExtendedLayer = false;
 		for (Layer* curLayer = m_FirstLayer; curLayer != nullptr; curLayer = curLayer->getNextLayer())
 		{
+			if (dataPtr > m_RawPacket->getRawData() + m_RawPacket->getRawDataLen())
+			{
+				PCPP_LOG_ERROR("Layer data pointer exceeds packet's boundary");
+				return false;
+			}
+
 			// set the data ptr
 			curLayer->m_Data = const_cast<uint8_t*>(dataPtr);
 
@@ -656,20 +662,28 @@ namespace pcpp
 		bool passedExtendedLayer = false;
 		while (curLayer != nullptr)
 		{
+			if (dataPtr > m_RawPacket->getRawData() + m_RawPacket->getRawDataLen())
+			{
+				PCPP_LOG_ERROR("Layer data pointer exceeds packet's boundary");
+				return false;
+			}
+
 			// set the data ptr
 			curLayer->m_Data = const_cast<uint8_t*>(dataPtr);
 
 			// set a flag if arrived to the layer being shortened
 			if (curLayer->getPrevLayer() == layer)
 				passedExtendedLayer = true;
 
+			size_t headerLen = curLayer->getHeaderLen();
+
 			// change the data length only for layers who come before the shortened layer. For layers who come after,
 			// data length isn't changed
 			if (!passedExtendedLayer)
 				curLayer->m_DataLen -= numOfBytesToShorten;
 
 			// assuming header length of the layer that requested to be extended hasn't been enlarged yet
-			size_t headerLen = curLayer->getHeaderLen() - (curLayer == layer ? numOfBytesToShorten : 0);
+			headerLen -= (curLayer == layer ? numOfBytesToShorten : 0);
 			dataPtr += headerLen;
 			curLayer = curLayer->getNextLayer();
 		}