Skip to content

Commit e9d3792

Browse files
danasanadanasana
committed
Relocate shortenLayer/extendLayer validity checks from Layer.cpp to BgpLayer.cpp
- Add two validation methods in BgpLayer.cpp: isValidShortenRange and isValidExtendRange - Perform validation before each extendLayer/shortenLayer call
1 parent 646610c commit e9d3792

File tree

3 files changed

+128
-109
lines changed

3 files changed

+128
-109
lines changed

‎Packet++/header/BgpLayer.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,10 @@ namespace pcpp
113113
}
114114

115115
void setBgpFields(size_t messageLen = 0);
116+
117+
bool isValidExtendRange(int offsetInLayer, size_t numOfBytesToExtend) const;
118+
119+
bool isValidShortenRange(int offsetInLayer, size_t numOfBytesToShorten) const;
116120
};
117121

118122
/// @class BgpOpenMessageLayer

‎Packet++/src/BgpLayer.cpp‎

Lines changed: 114 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
#include "Logger.h"
44
#include "BgpLayer.h"
5+
#include "Packet.h"
56
#include "EndianPortable.h"
67
#include "GeneralUtils.h"
78

@@ -117,6 +118,50 @@ namespace pcpp
117118
}
118119
}
119120

121+
bool BgpLayer::isValidExtendRange(int offsetInLayer, size_t numOfBytesToExtend) const
122+
{
123+
int rawPacketLen = m_Packet->getRawPacket()->getRawDataLen();
124+
const uint8_t* rawPacketPtr = m_Packet->getRawPacket()->getRawData();
125+
126+
if (m_Data - rawPacketPtr + static_cast<ptrdiff_t>(offsetInLayer) > static_cast<ptrdiff_t>(rawPacketLen))
127+
{
128+
PCPP_LOG_ERROR("Requested offset is larger than total packet length");
129+
return false;
130+
}
131+
132+
if (m_NextLayer != nullptr && static_cast<ptrdiff_t>(offsetInLayer) > m_NextLayer->getData() - m_Data)
133+
{
134+
PCPP_LOG_ERROR("Requested offset exceeds current layer's boundary");
135+
return false;
136+
}
137+
138+
return true;
139+
}
140+
141+
bool BgpLayer::isValidShortenRange(int offsetInLayer, size_t numOfBytesToShorten) const
142+
{
143+
int rawPacketLen = m_Packet->getRawPacket()->getRawDataLen();
144+
const uint8_t* rawPacketPtr = m_Packet->getRawPacket()->getRawData();
145+
146+
if (m_Data - rawPacketPtr + static_cast<ptrdiff_t>(offsetInLayer) +
147+
static_cast<ptrdiff_t>(numOfBytesToShorten) >
148+
static_cast<ptrdiff_t>(rawPacketLen))
149+
{
150+
PCPP_LOG_ERROR("Requested number of bytes to shorten is larger than total packet length");
151+
return false;
152+
}
153+
154+
if (m_NextLayer != nullptr &&
155+
static_cast<ptrdiff_t>(offsetInLayer) + static_cast<ptrdiff_t>(numOfBytesToShorten) >
156+
m_NextLayer->getData() - m_Data)
157+
{
158+
PCPP_LOG_ERROR("Requested number of bytes to shorten exceeds current layer's boundary");
159+
return false;
160+
}
161+
162+
return true;
163+
}
164+
120165
// ~~~~~~~~~~~~~~~~~~~~
121166
// BgpOpenMessageLayer
122167
// ~~~~~~~~~~~~~~~~~~~~
@@ -261,29 +306,34 @@ namespace pcpp
261306
uint8_t newOptionalParamsData[1500];
262307
size_t newOptionalParamsDataLen = optionalParamsToByteArray(optionalParameters, newOptionalParamsData, 1500);
263308
size_t curOptionalParamsDataLen = getOptionalParametersLength();
309+
int offsetInLayer = sizeof(bgp_open_message);
264310

265311
if (newOptionalParamsDataLen > curOptionalParamsDataLen)
266312
{
267-
bool res = extendLayer(sizeof(bgp_open_message), newOptionalParamsDataLen - curOptionalParamsDataLen);
268-
if (!res)
313+
size_t numOfBytesToExtend = newOptionalParamsDataLen - curOptionalParamsDataLen;
314+
315+
if (!isValidExtendRange(offsetInLayer, numOfBytesToExtend) ||
316+
!extendLayer(offsetInLayer, numOfBytesToExtend))
269317
{
270318
PCPP_LOG_ERROR("Couldn't extend BGP open layer to include the additional optional parameters");
271-
return res;
319+
return false;
272320
}
273321
}
274322
else if (newOptionalParamsDataLen < curOptionalParamsDataLen)
275323
{
276-
bool res = shortenLayer(sizeof(bgp_open_message), curOptionalParamsDataLen - newOptionalParamsDataLen);
277-
if (!res)
324+
size_t numOfBytesToShorten = curOptionalParamsDataLen - newOptionalParamsDataLen;
325+
326+
if (!isValidShortenRange(offsetInLayer, numOfBytesToShorten) ||
327+
!shortenLayer(offsetInLayer, numOfBytesToShorten))
278328
{
279329
PCPP_LOG_ERROR("Couldn't shorten BGP open layer to set the right size of the optional parameters data");
280-
return res;
330+
return false;
281331
}
282332
}
283333

284334
if (newOptionalParamsDataLen > 0)
285335
{
286-
memcpy(m_Data + sizeof(bgp_open_message), newOptionalParamsData, newOptionalParamsDataLen);
336+
memcpy(m_Data + offsetInLayer, newOptionalParamsData, newOptionalParamsDataLen);
287337
}
288338

289339
getOpenMsgHeader()->optionalParameterLength = (uint8_t)newOptionalParamsDataLen;
@@ -565,32 +615,34 @@ namespace pcpp
565615
uint8_t newWithdrawnRoutesData[1500];
566616
size_t newWithdrawnRoutesDataLen = prefixAndIPDataToByteArray(withdrawnRoutes, newWithdrawnRoutesData, 1500);
567617
size_t curWithdrawnRoutesDataLen = getWithdrawnRoutesLength();
618+
int offsetInLayer = sizeof(bgp_common_header) + sizeof(uint16_t);
568619

569620
if (newWithdrawnRoutesDataLen > curWithdrawnRoutesDataLen)
570621
{
571-
bool res = extendLayer(sizeof(bgp_common_header) + sizeof(uint16_t),
572-
newWithdrawnRoutesDataLen - curWithdrawnRoutesDataLen);
573-
if (!res)
622+
size_t numOfBytesToExtend = newWithdrawnRoutesDataLen - curWithdrawnRoutesDataLen;
623+
624+
if (!isValidExtendRange(offsetInLayer, numOfBytesToExtend) ||
625+
!extendLayer(offsetInLayer, numOfBytesToExtend))
574626
{
575-
PCPP_LOG_ERROR("Couldn't extend BGP update layer to include the additional withdrawn routes");
576-
return res;
627+
PCPP_LOG_ERROR("Couldn't extend BGP open layer to include the additional optional parameters");
628+
return false;
577629
}
578630
}
579631
else if (newWithdrawnRoutesDataLen < curWithdrawnRoutesDataLen)
580632
{
581-
bool res = shortenLayer(sizeof(bgp_common_header) + sizeof(uint16_t),
582-
curWithdrawnRoutesDataLen - newWithdrawnRoutesDataLen);
583-
if (!res)
633+
size_t numOfBytesToShorten = curWithdrawnRoutesDataLen - newWithdrawnRoutesDataLen;
634+
635+
if (!isValidShortenRange(offsetInLayer, numOfBytesToShorten) ||
636+
!shortenLayer(offsetInLayer, numOfBytesToShorten))
584637
{
585-
PCPP_LOG_ERROR("Couldn't shorten BGP update layer to set the right size of the withdrawn routes data");
586-
return res;
638+
PCPP_LOG_ERROR("Couldn't shorten BGP open layer to set the right size of the optional parameters data");
639+
return false;
587640
}
588641
}
589642

590643
if (newWithdrawnRoutesDataLen > 0)
591644
{
592-
memcpy(m_Data + sizeof(bgp_common_header) + sizeof(uint16_t), newWithdrawnRoutesData,
593-
newWithdrawnRoutesDataLen);
645+
memcpy(m_Data + offsetInLayer, newWithdrawnRoutesData, newWithdrawnRoutesDataLen);
594646
}
595647

596648
getBasicHeader()->length =
@@ -642,32 +694,34 @@ namespace pcpp
642694
size_t newPathAttributesDataLen = pathAttributesToByteArray(pathAttributes, newPathAttributesData, 1500);
643695
size_t curPathAttributesDataLen = getPathAttributesLength();
644696
size_t curWithdrawnRoutesDataLen = getWithdrawnRoutesLength();
697+
int offsetInLayer = sizeof(bgp_common_header) + 2 * sizeof(uint16_t) + curWithdrawnRoutesDataLen;
645698

646699
if (newPathAttributesDataLen > curPathAttributesDataLen)
647700
{
648-
bool res = extendLayer(sizeof(bgp_common_header) + 2 * sizeof(uint16_t) + curWithdrawnRoutesDataLen,
649-
newPathAttributesDataLen - curPathAttributesDataLen);
650-
if (!res)
701+
size_t numOfBytesToExtend = newPathAttributesDataLen - curPathAttributesDataLen;
702+
703+
if (!isValidExtendRange(offsetInLayer, numOfBytesToExtend) ||
704+
!extendLayer(offsetInLayer, numOfBytesToExtend))
651705
{
652-
PCPP_LOG_ERROR("Couldn't extend BGP update layer to include the additional path attributes");
653-
return res;
706+
PCPP_LOG_ERROR("Couldn't extend BGP open layer to include the additional optional parameters");
707+
return false;
654708
}
655709
}
656710
else if (newPathAttributesDataLen < curPathAttributesDataLen)
657711
{
658-
bool res = shortenLayer(sizeof(bgp_common_header) + 2 * sizeof(uint16_t) + curWithdrawnRoutesDataLen,
659-
curPathAttributesDataLen - newPathAttributesDataLen);
660-
if (!res)
712+
size_t numOfBytesToShorten = curPathAttributesDataLen - newPathAttributesDataLen;
713+
714+
if (!isValidShortenRange(offsetInLayer, numOfBytesToShorten) ||
715+
!shortenLayer(offsetInLayer, numOfBytesToShorten))
661716
{
662-
PCPP_LOG_ERROR("Couldn't shorten BGP update layer to set the right size of the path attributes data");
663-
return res;
717+
PCPP_LOG_ERROR("Couldn't shorten BGP open layer to set the right size of the optional parameters data");
718+
return false;
664719
}
665720
}
666721

667722
if (newPathAttributesDataLen > 0)
668723
{
669-
memcpy(m_Data + sizeof(bgp_common_header) + 2 * sizeof(uint16_t) + curWithdrawnRoutesDataLen,
670-
newPathAttributesData, newPathAttributesDataLen);
724+
memcpy(m_Data + offsetInLayer, newPathAttributesData, newPathAttributesDataLen);
671725
}
672726

673727
getBasicHeader()->length =
@@ -741,35 +795,35 @@ namespace pcpp
741795
size_t curNlriDataLen = getNetworkLayerReachabilityInfoLength();
742796
size_t curPathAttributesDataLen = getPathAttributesLength();
743797
size_t curWithdrawnRoutesDataLen = getWithdrawnRoutesLength();
798+
int offsetInLayer =
799+
sizeof(bgp_common_header) + 2 * sizeof(uint16_t) + curWithdrawnRoutesDataLen + curPathAttributesDataLen;
744800

745801
if (newNlriDataLen > curNlriDataLen)
746802
{
747-
bool res = extendLayer(sizeof(bgp_common_header) + 2 * sizeof(uint16_t) + curWithdrawnRoutesDataLen +
748-
curPathAttributesDataLen,
749-
newNlriDataLen - curNlriDataLen);
750-
if (!res)
803+
size_t numOfBytesToExtend = newNlriDataLen - curNlriDataLen;
804+
805+
if (!isValidExtendRange(offsetInLayer, numOfBytesToExtend) ||
806+
!extendLayer(offsetInLayer, numOfBytesToExtend))
751807
{
752-
PCPP_LOG_ERROR("Couldn't extend BGP update layer to include the additional NLRI data");
753-
return res;
808+
PCPP_LOG_ERROR("Couldn't extend BGP open layer to include the additional optional parameters");
809+
return false;
754810
}
755811
}
756812
else if (newNlriDataLen < curNlriDataLen)
757813
{
758-
bool res = shortenLayer(sizeof(bgp_common_header) + 2 * sizeof(uint16_t) + curWithdrawnRoutesDataLen +
759-
curPathAttributesDataLen,
760-
curNlriDataLen - newNlriDataLen);
761-
if (!res)
814+
size_t numOfBytesToShorten = curNlriDataLen - newNlriDataLen;
815+
816+
if (!isValidShortenRange(offsetInLayer, numOfBytesToShorten) ||
817+
!shortenLayer(offsetInLayer, numOfBytesToShorten))
762818
{
763-
PCPP_LOG_ERROR("Couldn't shorten BGP update layer to set the right size of the NLRI data");
764-
return res;
819+
PCPP_LOG_ERROR("Couldn't shorten BGP open layer to set the right size of the optional parameters data");
820+
return false;
765821
}
766822
}
767823

768824
if (newNlriDataLen > 0)
769825
{
770-
memcpy(m_Data + sizeof(bgp_common_header) + 2 * sizeof(uint16_t) + curWithdrawnRoutesDataLen +
771-
curPathAttributesDataLen,
772-
newNlriData, newNlriDataLen);
826+
memcpy(m_Data + offsetInLayer, newNlriData, newNlriDataLen);
773827
}
774828

775829
getBasicHeader()->length = htobe16(be16toh(getBasicHeader()->length) + newNlriDataLen - curNlriDataLen);
@@ -866,30 +920,34 @@ namespace pcpp
866920
}
867921

868922
size_t curNotificationDataLen = getNotificationDataLen();
923+
int offsetInLayer = sizeof(bgp_notification_message);
869924

870925
if (newNotificationDataLen > curNotificationDataLen)
871926
{
872-
bool res = extendLayer(sizeof(bgp_notification_message), newNotificationDataLen - curNotificationDataLen);
873-
if (!res)
927+
size_t numOfBytesToExtend = newNotificationDataLen - curNotificationDataLen;
928+
929+
if (!isValidExtendRange(offsetInLayer, numOfBytesToExtend) ||
930+
!extendLayer(offsetInLayer, numOfBytesToExtend))
874931
{
875-
PCPP_LOG_ERROR("Couldn't extend BGP notification layer to include the additional notification data");
876-
return res;
932+
PCPP_LOG_ERROR("Couldn't extend BGP open layer to include the additional optional parameters");
933+
return false;
877934
}
878935
}
879936
else if (newNotificationDataLen < curNotificationDataLen)
880937
{
881-
bool res = shortenLayer(sizeof(bgp_notification_message), curNotificationDataLen - newNotificationDataLen);
882-
if (!res)
938+
size_t numOfBytesToShorten = curNotificationDataLen - newNotificationDataLen;
939+
940+
if (!isValidShortenRange(offsetInLayer, numOfBytesToShorten) ||
941+
!shortenLayer(offsetInLayer, numOfBytesToShorten))
883942
{
884-
PCPP_LOG_ERROR(
885-
"Couldn't shorten BGP notification layer to set the right size of the notification data");
886-
return res;
943+
PCPP_LOG_ERROR("Couldn't shorten BGP open layer to set the right size of the optional parameters data");
944+
return false;
887945
}
888946
}
889947

890948
if (newNotificationDataLen > 0)
891949
{
892-
memcpy(m_Data + sizeof(bgp_notification_message), newNotificationData, newNotificationDataLen);
950+
memcpy(m_Data + offsetInLayer, newNotificationData, newNotificationDataLen);
893951
}
894952

895953
getNotificationMsgHeader()->length = htobe16(sizeof(bgp_notification_message) + newNotificationDataLen);

‎Packet++/src/Layer.cpp‎

Lines changed: 10 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -61,20 +61,13 @@ namespace pcpp
6161
return false;
6262
}
6363

64-
if (offsetInLayer < 0)
65-
{
66-
PCPP_LOG_ERROR("Requested offset is negative");
67-
return false;
68-
}
69-
70-
if (static_cast<size_t>(offsetInLayer) > m_DataLen)
71-
{
72-
PCPP_LOG_ERROR("Requested offset is larger than data length");
73-
return false;
74-
}
75-
7664
if (m_Packet == nullptr)
7765
{
66+
if (static_cast<size_t>(offsetInLayer) > m_DataLen)
67+
{
68+
PCPP_LOG_ERROR("Requested offset is larger than data length");
69+
return false;
70+
}
7871
uint8_t* newData = new uint8_t[m_DataLen + numOfBytesToExtend];
7972
memcpy(newData, m_Data, offsetInLayer);
8073
memcpy(newData + offsetInLayer + numOfBytesToExtend, m_Data + offsetInLayer, m_DataLen - offsetInLayer);
@@ -84,19 +77,6 @@ namespace pcpp
8477
return true;
8578
}
8679

87-
if (m_Data - m_Packet->m_RawPacket->getRawData() + static_cast<ptrdiff_t>(offsetInLayer) >
88-
static_cast<ptrdiff_t>(m_Packet->m_RawPacket->getRawDataLen()))
89-
{
90-
PCPP_LOG_ERROR("Requested offset is larger than total packet length");
91-
return false;
92-
}
93-
94-
if (m_NextLayer != nullptr && static_cast<ptrdiff_t>(offsetInLayer) > m_NextLayer->m_Data - m_Data)
95-
{
96-
PCPP_LOG_ERROR("Requested offset exceeds current layer's boundary");
97-
return false;
98-
}
99-
10080
return m_Packet->extendLayer(this, offsetInLayer, numOfBytesToExtend);
10181
}
10282

@@ -108,20 +88,13 @@ namespace pcpp
10888
return false;
10989
}
11090

111-
if (offsetInLayer < 0)
112-
{
113-
PCPP_LOG_ERROR("Requested offset is negative");
114-
return false;
115-
}
116-
117-
if (static_cast<size_t>(offsetInLayer) >= m_DataLen)
118-
{
119-
PCPP_LOG_ERROR("Requested offset is larger than data length");
120-
return false;
121-
}
122-
12391
if (m_Packet == nullptr)
12492
{
93+
if (static_cast<size_t>(offsetInLayer) >= m_DataLen)
94+
{
95+
PCPP_LOG_ERROR("Requested offset is larger than data length");
96+
return false;
97+
}
12598
uint8_t* newData = new uint8_t[m_DataLen - numOfBytesToShorten];
12699
memcpy(newData, m_Data, offsetInLayer);
127100
memcpy(newData + offsetInLayer, m_Data + offsetInLayer + numOfBytesToShorten,
@@ -138,22 +111,6 @@ namespace pcpp
138111
return false;
139112
}
140113

141-
if (m_Data - m_Packet->m_RawPacket->getRawData() + static_cast<ptrdiff_t>(offsetInLayer) +
142-
static_cast<ptrdiff_t>(numOfBytesToShorten) >
143-
static_cast<ptrdiff_t>(m_Packet->m_RawPacket->getRawDataLen()))
144-
{
145-
PCPP_LOG_ERROR("Requested number of bytes to shorten is larger than total packet length");
146-
return false;
147-
}
148-
149-
if (m_NextLayer != nullptr &&
150-
static_cast<ptrdiff_t>(offsetInLayer) + static_cast<ptrdiff_t>(numOfBytesToShorten) >
151-
m_NextLayer->m_Data - m_Data)
152-
{
153-
PCPP_LOG_ERROR("Requested number of bytes to shorten exceeds current layer's boundary");
154-
return false;
155-
}
156-
157114
return m_Packet->shortenLayer(this, offsetInLayer, numOfBytesToShorten);
158115
}
159116

0 commit comments

Comments
 (0)