Skip to content

Commit bbc57b2

Browse files
tony-josi-awsarchigup
authored andcommitted
Fix CWE-126/476/822: IPv6 buffer validation and bounds checking
- Fix missing NULL pointer checks in FreeRTOS_ND.c - Add ICMPv6 minimum packet size validation - Move IPv6 version field check after buffer validation - Fix integer wrap-around in IPv6 payload length processing - Add unit test coverage - MISRA compliance fixes Cherry-picked from PR FreeRTOS#1296 (V4.3.4): 3fabc55 Initial version with UTs fixed 55705f0 Fix missing check for IPv6 version field in header 5af6088 Fix integer wrap around with IPv6 payload length field processing 442dda7 Fix coverage and spell check e659f7f Minor fix c8f8926 Fix formatting b679d2b Fix MISRA issues
1 parent 50c82f7 commit bbc57b2

10 files changed

Lines changed: 503 additions & 191 deletions

File tree

source/FreeRTOS_IPv6.c

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,6 @@ const struct xIPv6_Address FreeRTOS_in6addr_loopback = { { 0U, 0U, 0U, 0U, 0U, 0
9393
size_t uxBufferLength )
9494
{
9595
BaseType_t xResult = pdFAIL;
96-
uint16_t ucVersionTrafficClass;
9796
uint16_t usPayloadLength;
9897
uint8_t ucNextHeader;
9998
size_t uxMinimumLength;
@@ -116,15 +115,6 @@ const struct xIPv6_Address FreeRTOS_in6addr_loopback = { { 0U, 0U, 0U, 0U, 0U, 0
116115
break;
117116
}
118117

119-
ucVersionTrafficClass = pxIPv6Packet->xIPHeader.ucVersionTrafficClass;
120-
121-
/* Test if the IP-version is 6. */
122-
if( ( ( ucVersionTrafficClass & ( uint8_t ) 0xF0U ) >> 4 ) != 6U )
123-
{
124-
DEBUG_SET_TRACE_VARIABLE( xLocation, 2 );
125-
break;
126-
}
127-
128118
/* Check if the IPv6-header is transferred. */
129119
if( uxBufferLength < ( ipSIZE_OF_ETH_HEADER + ipSIZE_OF_IPv6_HEADER ) )
130120
{
@@ -497,6 +487,7 @@ eFrameProcessingResult_t prvAllowIPPacketIPv6( const IPHeader_IPv6_t * const pxI
497487
const IPv6_Address_t * pxDestinationIPAddress = &( pxIPv6Header->xDestinationAddress );
498488
const IPv6_Address_t * pxSourceIPAddress = &( pxIPv6Header->xSourceAddress );
499489
BaseType_t xHasUnspecifiedAddress = pdFALSE;
490+
uint16_t ucVersionTrafficClass = pxIPv6Header->ucVersionTrafficClass;
500491

501492
/* Drop if packet has unspecified IPv6 address (defined in RFC4291 - sec 2.5.2)
502493
* either in source or destination address. */
@@ -506,10 +497,17 @@ eFrameProcessingResult_t prvAllowIPPacketIPv6( const IPHeader_IPv6_t * const pxI
506497
xHasUnspecifiedAddress = pdTRUE;
507498
}
508499

500+
/* Test if the IP-version is 6. */
501+
if( ( ( ucVersionTrafficClass & ( uint8_t ) 0xF0U ) >> 4 ) != 6U )
502+
{
503+
/* Can not handle, unknown or invalid header version. */
504+
eReturn = eReleaseBuffer;
505+
FreeRTOS_printf( ( "prvAllowIPPacketIPv6: drop packet, invalid header version: %u\n", ( ucVersionTrafficClass & ( uint8_t ) 0xF0U ) >> 4 ) );
506+
}
509507
/* Is the packet for this IP address? */
510-
if( ( xHasUnspecifiedAddress == pdFALSE ) &&
511-
( pxNetworkBuffer->pxEndPoint != NULL ) &&
512-
( memcmp( pxDestinationIPAddress->ucBytes, pxNetworkBuffer->pxEndPoint->ipv6_settings.xIPAddress.ucBytes, sizeof( IPv6_Address_t ) ) == 0 ) )
508+
else if( ( xHasUnspecifiedAddress == pdFALSE ) &&
509+
( pxNetworkBuffer->pxEndPoint != NULL ) &&
510+
( memcmp( pxDestinationIPAddress->ucBytes, pxNetworkBuffer->pxEndPoint->ipv6_settings.xIPAddress.ucBytes, sizeof( IPv6_Address_t ) ) == 0 ) )
513511
{
514512
eReturn = eProcessBuffer;
515513
}

source/FreeRTOS_IPv6_Utils.c

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ BaseType_t prvChecksumIPv6Checks( uint8_t * pucEthernetBuffer,
9292
{
9393
uxExtensionHeaderLength = usGetExtensionHeaderLength( pucEthernetBuffer, uxBufferLength, &pxSet->ucProtocol );
9494

95-
if( uxExtensionHeaderLength >= uxBufferLength )
95+
if( ( ipSIZE_OF_ETH_HEADER + ipSIZE_OF_IPv6_HEADER + uxExtensionHeaderLength ) >= uxBufferLength )
9696
{
9797
/* Error detected when parsing extension header. */
9898
pxSet->usChecksum = ipINVALID_LENGTH;
@@ -107,8 +107,18 @@ BaseType_t prvChecksumIPv6Checks( uint8_t * pucEthernetBuffer,
107107
/* coverity[misra_c_2012_rule_11_3_violation] */
108108
pxSet->pxProtocolHeaders = ( ( ProtocolHeaders_t * ) &( pucEthernetBuffer[ ipSIZE_OF_ETH_HEADER + ipSIZE_OF_IPv6_HEADER + uxExtensionHeaderLength ] ) );
109109
pxSet->usPayloadLength = FreeRTOS_ntohs( pxSet->pxIPPacket_IPv6->usPayloadLength );
110+
110111
/* For IPv6, the number of bytes in the protocol is indicated. */
111-
pxSet->usProtocolBytes = ( uint16_t ) ( pxSet->usPayloadLength - uxExtensionHeaderLength );
112+
if( pxSet->usPayloadLength < uxExtensionHeaderLength )
113+
{
114+
/* Invalid payload length - extension headers exceed payload. */
115+
pxSet->usChecksum = ipINVALID_LENGTH;
116+
xReturn = 4;
117+
}
118+
else
119+
{
120+
pxSet->usProtocolBytes = ( uint16_t ) ( pxSet->usPayloadLength - uxExtensionHeaderLength );
121+
}
112122

113123
uxNeeded = ( size_t ) pxSet->usPayloadLength;
114124
uxNeeded += ipSIZE_OF_ETH_HEADER + ipSIZE_OF_IPv6_HEADER;

source/FreeRTOS_ND.c

Lines changed: 216 additions & 163 deletions
Large diffs are not rendered by default.

source/include/FreeRTOS_IPv6_Private.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,8 @@
132132
struct xNetworkEndPoint;
133133
struct xNetworkInterface;
134134

135+
#define ipICMPv6_GENERAL_FIELD_SIZE ( 4U )
136+
135137
#include "pack_struct_start.h"
136138
struct xIP_HEADER_IPv6
137139
{

test/cbmc/proofs/ND/prvProcessICMPMessage_IPv6/ProcessICMPMessage_IPv6_harness.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,9 @@ void harness()
137137
uint16_t usEthernetBufferSize;
138138
NetworkBufferDescriptor_t * pxLocalARPWaitingNetworkBuffer;
139139

140-
__CPROVER_assume( ( ulLen >= sizeof( ICMPPacket_IPv6_t ) ) && ( ulLen < ipconfigNETWORK_MTU ) );
140+
/* Following assumption is to make sure ulLen doesn't go
141+
* beyond CBMC_MAX_OBJECT_SIZE */
142+
__CPROVER_assume( ulLen < ( CBMC_MAX_OBJECT_SIZE - ipBUFFER_PADDING ) );
141143

142144
pxNetworkBuffer = pxGetNetworkBufferWithDescriptor( ulLen, 0 );
143145

test/cbmc/proofs/prvChecksumIPv6Checks/prvChecksumIPv6Checks_harness.c

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,19 +21,26 @@ void harness()
2121
size_t uxBufferSize;
2222
uint8_t * pucEthernetBuffer;
2323
struct xPacketSummary xSet;
24+
BaseType_t xReturn;
2425

2526
/* We must have ethernet header to get frame type. */
2627
__CPROVER_assume( uxBufferSize >= sizeof( IPPacket_IPv6_t ) && uxBufferSize <= ipconfigNETWORK_MTU );
2728

2829
/* Ethernet buffer is not possible to be NULL. */
2930
pucEthernetBuffer = ( uint8_t * ) safeMalloc( uxBufferSize );
3031
__CPROVER_assume( pucEthernetBuffer != NULL );
32+
__CPROVER_havoc_object( pucEthernetBuffer );
3133

3234
/* This is set before calling prvChecksumIPv6Checks. */
3335
xSet.pxIPPacket = ( const IPPacket_t * ) pucEthernetBuffer;
3436
xSet.pxIPPacket_IPv6 = ( const IPHeader_IPv6_t * ) ( pucEthernetBuffer + ipSIZE_OF_ETH_HEADER );
3537

36-
prvChecksumIPv6Checks( pucEthernetBuffer,
37-
uxBufferSize,
38-
&xSet );
38+
xReturn = prvChecksumIPv6Checks( pucEthernetBuffer,
39+
uxBufferSize,
40+
&xSet );
41+
42+
if( xReturn == 0 )
43+
{
44+
__CPROVER_assert( ( xSet.usProtocolBytes <= FreeRTOS_ntohs( xSet.pxIPPacket_IPv6->usPayloadLength ) ), "xSet.usProtocolBytes shouldn't be greater than IPv6 usPayloadLength" );
45+
}
3946
}

test/unit-test/FreeRTOS_IPv6/FreeRTOS_IPv6_utest.c

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ void test_prvAllowIPPacketIPv6_SourceUnspecifiedAddress()
6868
memset( &xIPv6Address, 0, sizeof( xIPv6Address ) );
6969
memcpy( xIPv6Address.xDestinationAddress.ucBytes, xIPAddressFive.ucBytes, ipSIZE_OF_IPv6_ADDRESS );
7070
memcpy( xIPv6Address.xSourceAddress.ucBytes, FreeRTOS_in6addr_any.ucBytes, ipSIZE_OF_IPv6_ADDRESS );
71+
xIPv6Address.ucVersionTrafficClass = 0x60U;
7172

7273
eResult = prvAllowIPPacketIPv6( &xIPv6Address, NULL, 0U );
7374
TEST_ASSERT_EQUAL( eReleaseBuffer, eResult );
@@ -85,6 +86,7 @@ void test_prvAllowIPPacketIPv6_DestinationUnspecifiedAddress()
8586
memset( &xIPv6Address, 0, sizeof( xIPv6Address ) );
8687
memcpy( xIPv6Address.xDestinationAddress.ucBytes, FreeRTOS_in6addr_any.ucBytes, ipSIZE_OF_IPv6_ADDRESS );
8788
memcpy( xIPv6Address.xSourceAddress.ucBytes, xIPAddressFive.ucBytes, ipSIZE_OF_IPv6_ADDRESS );
89+
xIPv6Address.ucVersionTrafficClass = 0x60U;
8890

8991
eResult = prvAllowIPPacketIPv6( &xIPv6Address, NULL, 0U );
9092
TEST_ASSERT_EQUAL( eReleaseBuffer, eResult );
@@ -100,6 +102,8 @@ void test_prvAllowIPPacketIPv6_HappyPath()
100102
NetworkBufferDescriptor_t * pxNetworkBuffer = prvInitializeNetworkDescriptor();
101103
TCPPacket_IPv6_t * pxTCPPacket = ( TCPPacket_IPv6_t * ) pxNetworkBuffer->pucEthernetBuffer;
102104

105+
pxTCPPacket->xIPHeader.ucVersionTrafficClass = 0x60U;
106+
103107
FreeRTOS_FindEndPointOnMAC_ExpectAndReturn( &pxTCPPacket->xEthernetHeader.xSourceAddress, NULL, NULL );
104108
usGenerateProtocolChecksum_ExpectAndReturn( pxNetworkBuffer->pucEthernetBuffer, pxNetworkBuffer->xDataLength, pdFALSE, ipCORRECT_CRC );
105109

@@ -118,6 +122,7 @@ void test_prvAllowIPPacketIPv6_MulticastAddress()
118122
/* Multicast IPv6 address is FF02::1 */
119123
IPv6_Address_t xMCIPAddress = { 0xFF, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01 };
120124

125+
pxTCPPacket->xIPHeader.ucVersionTrafficClass = 0x60U;
121126
memcpy( pxTCPPacket->xIPHeader.xDestinationAddress.ucBytes, xMCIPAddress.ucBytes, ipSIZE_OF_IPv6_ADDRESS );
122127

123128
FreeRTOS_FindEndPointOnIP_IPv6_ExpectAndReturn( &( pxTCPPacket->xIPHeader.xSourceAddress ), pxNetworkBuffer->pxEndPoint );
@@ -139,6 +144,7 @@ void test_prvAllowIPPacketIPv6_LoopbackAddress()
139144
TCPPacket_IPv6_t * pxTCPPacket = ( TCPPacket_IPv6_t * ) pxNetworkBuffer->pucEthernetBuffer;
140145
NetworkEndPoint_t xEndPoint;
141146

147+
pxTCPPacket->xIPHeader.ucVersionTrafficClass = 0x60U;
142148
memcpy( pxTCPPacket->xIPHeader.xSourceAddress.ucBytes, FreeRTOS_in6addr_loopback.ucBytes, ipSIZE_OF_IPv6_ADDRESS );
143149
memcpy( pxTCPPacket->xIPHeader.xDestinationAddress.ucBytes, FreeRTOS_in6addr_loopback.ucBytes, ipSIZE_OF_IPv6_ADDRESS );
144150

@@ -163,6 +169,7 @@ void test_prvAllowIPPacketIPv6_LoopbackNotMatchDest()
163169
NetworkBufferDescriptor_t * pxNetworkBuffer = prvInitializeNetworkDescriptor();
164170
TCPPacket_IPv6_t * pxTCPPacket = ( TCPPacket_IPv6_t * ) pxNetworkBuffer->pucEthernetBuffer;
165171

172+
pxTCPPacket->xIPHeader.ucVersionTrafficClass = 0x60U;
166173
pxTCPPacket->xIPHeader.xDestinationAddress.ucBytes[ 15 ] = 0x11;
167174

168175
FreeRTOS_FindEndPointOnIP_IPv6_ExpectAndReturn( &pxTCPPacket->xIPHeader.xSourceAddress, pxNetworkBuffer->pxEndPoint );
@@ -185,6 +192,7 @@ void test_prvAllowIPPacketIPv6_LoopbackNotMatchSrc()
185192
TCPPacket_IPv6_t * pxTCPPacket = ( TCPPacket_IPv6_t * ) pxNetworkBuffer->pucEthernetBuffer;
186193
NetworkEndPoint_t xEndPoint;
187194

195+
pxTCPPacket->xIPHeader.ucVersionTrafficClass = 0x60U;
188196
memcpy( pxTCPPacket->xIPHeader.xDestinationAddress.ucBytes, FreeRTOS_in6addr_loopback.ucBytes, ipSIZE_OF_IPv6_ADDRESS );
189197

190198
FreeRTOS_FindEndPointOnIP_IPv6_ExpectAndReturn( &pxTCPPacket->xIPHeader.xSourceAddress, &xEndPoint );
@@ -206,6 +214,8 @@ void test_prvAllowIPPacketIPv6_NetworkDown()
206214

207215
pxNetworkBuffer->pxEndPoint = NULL;
208216

217+
pxTCPPacket->xIPHeader.ucVersionTrafficClass = 0x60U;
218+
209219
FreeRTOS_FindEndPointOnIP_IPv6_ExpectAndReturn( &pxTCPPacket->xIPHeader.xSourceAddress, NULL );
210220
FreeRTOS_IsNetworkUp_IgnoreAndReturn( 0 );
211221
FreeRTOS_FindEndPointOnMAC_ExpectAndReturn( &pxTCPPacket->xEthernetHeader.xSourceAddress, NULL, NULL );
@@ -225,6 +235,8 @@ void test_prvAllowIPPacketIPv6_SelfSend()
225235
NetworkBufferDescriptor_t * pxNetworkBuffer = prvInitializeNetworkDescriptor();
226236
TCPPacket_IPv6_t * pxTCPPacket = ( TCPPacket_IPv6_t * ) pxNetworkBuffer->pucEthernetBuffer;
227237

238+
pxTCPPacket->xIPHeader.ucVersionTrafficClass = 0x60U;
239+
228240
FreeRTOS_FindEndPointOnMAC_ExpectAndReturn( &pxTCPPacket->xEthernetHeader.xSourceAddress, NULL, pxNetworkBuffer->pxEndPoint );
229241

230242
eResult = prvAllowIPPacketIPv6( &pxTCPPacket->xIPHeader, pxNetworkBuffer, 0U );
@@ -241,6 +253,8 @@ void test_prvAllowIPPacketIPv6_ChecksumError()
241253
NetworkBufferDescriptor_t * pxNetworkBuffer = prvInitializeNetworkDescriptor();
242254
TCPPacket_IPv6_t * pxTCPPacket = ( TCPPacket_IPv6_t * ) pxNetworkBuffer->pucEthernetBuffer;
243255

256+
pxTCPPacket->xIPHeader.ucVersionTrafficClass = 0x60U;
257+
244258
FreeRTOS_FindEndPointOnMAC_ExpectAndReturn( &pxTCPPacket->xEthernetHeader.xSourceAddress, NULL, NULL );
245259
usGenerateProtocolChecksum_ExpectAndReturn( pxNetworkBuffer->pucEthernetBuffer, pxNetworkBuffer->xDataLength, pdFALSE, ipWRONG_CRC );
246260

@@ -260,6 +274,8 @@ void test_prvAllowIPPacketIPv6_InvalidPacket()
260274

261275
pxNetworkBuffer->pxEndPoint = NULL;
262276

277+
pxTCPPacket->xIPHeader.ucVersionTrafficClass = 0x60U;
278+
263279
FreeRTOS_FindEndPointOnIP_IPv6_ExpectAndReturn( &pxTCPPacket->xIPHeader.xSourceAddress, NULL );
264280
FreeRTOS_IsNetworkUp_IgnoreAndReturn( 1 );
265281

@@ -285,6 +301,8 @@ void test_prvAllowIPPacketIPv6_EndpointDifferentAddress()
285301
memcpy( xEndpoint.ipv6_settings.xIPAddress.ucBytes, xDiffIPAddress.ucBytes, ipSIZE_OF_IPv6_ADDRESS );
286302
pxNetworkBuffer->pxEndPoint = &xEndpoint;
287303

304+
pxTCPPacket->xIPHeader.ucVersionTrafficClass = 0x60U;
305+
288306
FreeRTOS_FindEndPointOnIP_IPv6_ExpectAndReturn( &( pxTCPPacket->xIPHeader.xSourceAddress ), NULL );
289307
FreeRTOS_IsNetworkUp_IgnoreAndReturn( 1 );
290308

test/unit-test/FreeRTOS_IPv6_Utils/FreeRTOS_IPv6_Utils_utest.c

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ void test_prvChecksumIPv6Checks_IncompleteIPv6Packet( void )
131131
BaseType_t usReturn;
132132
uint8_t pucEthernetBuffer[ ipconfigTCPv6_MSS ];
133133
IPHeader_IPv6_t * pxIPv6Packet;
134-
size_t uxBufferLength = ipSIZE_OF_ETH_HEADER + ipSIZE_OF_IPv6_HEADER;
134+
size_t uxBufferLength = ipSIZE_OF_ETH_HEADER + ipSIZE_OF_IPv6_HEADER + 4;
135135
struct xPacketSummary xSet;
136136

137137
memset( pucEthernetBuffer, 0, ipconfigTCPv6_MSS );
@@ -205,6 +205,47 @@ void test_prvChecksumIPv6Checks_LargeExtensionHeader( void )
205205
TEST_ASSERT_EQUAL( 3, usReturn );
206206
}
207207

208+
/**
209+
* @brief Prepare a packet with large extension header length.
210+
* - ipIPv6_EXT_HEADER_ROUTING_HEADER
211+
* - ipIPv6_EXT_HEADER_HOP_BY_HOP
212+
* - ipPROTOCOL_TCP
213+
*/
214+
void test_prvChecksumIPv6Checks_IncorrectPayloadLength( void )
215+
{
216+
BaseType_t usReturn;
217+
struct xPacketSummary xSet;
218+
NetworkBufferDescriptor_t * pxNetworkBuffer = prvInitializeNetworkDescriptorWithExtensionHeader( ipPROTOCOL_TCP );
219+
IPHeader_IPv6_t * pxIPv6Header = ( IPHeader_IPv6_t * ) &( pxNetworkBuffer->pucEthernetBuffer[ ipSIZE_OF_ETH_HEADER ] );
220+
size_t uxIndex = ipSIZE_OF_ETH_HEADER + ipSIZE_OF_IPv6_HEADER;
221+
222+
/* Modify the extension header */
223+
pxIPv6Header->ucNextHeader = ipIPv6_EXT_HEADER_HOP_BY_HOP;
224+
pxNetworkBuffer->pucEthernetBuffer[ uxIndex ] = ipIPv6_EXT_HEADER_ROUTING_HEADER;
225+
pxNetworkBuffer->pucEthernetBuffer[ uxIndex + 1 ] = 5U; /* Extension header length is set to 200*8 + 8, which is larger than buffer size. */
226+
uxIndex += 8;
227+
pxNetworkBuffer->pucEthernetBuffer[ uxIndex ] = ipPROTOCOL_TCP;
228+
pxNetworkBuffer->pucEthernetBuffer[ uxIndex + 1 ] = 0;
229+
uxIndex += 8;
230+
231+
xSet.pxIPPacket_IPv6 = ( ( const IPHeader_IPv6_t * ) pxNetworkBuffer->pucEthernetBuffer );
232+
IPHeader_IPv6_t * pxIPPacket_IPv6 = ( IPHeader_IPv6_t * ) pxNetworkBuffer->pucEthernetBuffer;
233+
234+
/* Incorrect payload length */
235+
pxIPPacket_IPv6->usPayloadLength = FreeRTOS_ntohs( 20 );
236+
237+
xGetExtensionOrder_ExpectAndReturn( ipIPv6_EXT_HEADER_HOP_BY_HOP, 0U, 1 );
238+
xGetExtensionOrder_ExpectAndReturn( ipIPv6_EXT_HEADER_HOP_BY_HOP, ipIPv6_EXT_HEADER_ROUTING_HEADER, 1 );
239+
xGetExtensionOrder_ExpectAndReturn( ipIPv6_EXT_HEADER_ROUTING_HEADER, ipPROTOCOL_TCP, 2 );
240+
xGetExtensionOrder_ExpectAndReturn( ipIPv6_EXT_HEADER_ROUTING_HEADER, ipPROTOCOL_TCP, 2 );
241+
242+
usReturn = prvChecksumIPv6Checks( pxNetworkBuffer->pucEthernetBuffer, pxNetworkBuffer->xDataLength, &xSet );
243+
244+
TEST_ASSERT_EQUAL( 4, usReturn );
245+
}
246+
247+
248+
208249
/**
209250
* @brief Prepare a packet have extension with following order.
210251
* - ipIPv6_EXT_HEADER_ROUTING_HEADER

0 commit comments

Comments
 (0)