Skip to content

Commit fc68874

Browse files
archiguparchigup
committed
Remove MAC-based loopback validation bypass
The IPv4 and IPv6 receive paths skipped all checksum validation when the source MAC matched the device's own MAC address. Since MAC addresses are spoofable on a local network, remove the bypass so that checksums are always verified.
1 parent 627d9bb commit fc68874

4 files changed

Lines changed: 28 additions & 58 deletions

File tree

‎source/FreeRTOS_IPv4.c‎

Lines changed: 21 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -487,34 +487,28 @@ enum eFrameProcessingResult prvAllowIPPacketIPv4( const struct xIP_PACKET * cons
487487
* define, so that the checksum won't be checked again here */
488488
if( eReturn == eProcessBuffer )
489489
{
490-
const NetworkEndPoint_t * pxEndPoint = FreeRTOS_FindEndPointOnMAC( &( pxIPPacket->xEthernetHeader.xSourceAddress ), NULL );
491-
492-
/* Do not check the checksum of loop-back messages. */
493-
if( pxEndPoint == NULL )
490+
/* Is the IP header checksum correct?
491+
*
492+
* NOTE: When the checksum of IP header is calculated while not omitting
493+
* the checksum field, the resulting value of the checksum always is 0xffff
494+
* which is denoted by ipCORRECT_CRC. See this wiki for more information:
495+
* https://en.wikipedia.org/wiki/IPv4_header_checksum#Verifying_the_IPv4_header_checksum
496+
* and this RFC: https://tools.ietf.org/html/rfc1624#page-4
497+
*/
498+
if( usGenerateChecksum( 0U, ( const uint8_t * ) &( pxIPHeader->ucVersionHeaderLength ), ( size_t ) uxHeaderLength ) != ipCORRECT_CRC )
494499
{
495-
/* Is the IP header checksum correct?
496-
*
497-
* NOTE: When the checksum of IP header is calculated while not omitting
498-
* the checksum field, the resulting value of the checksum always is 0xffff
499-
* which is denoted by ipCORRECT_CRC. See this wiki for more information:
500-
* https://en.wikipedia.org/wiki/IPv4_header_checksum#Verifying_the_IPv4_header_checksum
501-
* and this RFC: https://tools.ietf.org/html/rfc1624#page-4
502-
*/
503-
if( usGenerateChecksum( 0U, ( const uint8_t * ) &( pxIPHeader->ucVersionHeaderLength ), ( size_t ) uxHeaderLength ) != ipCORRECT_CRC )
504-
{
505-
/* Check sum in IP-header not correct. */
506-
eReturn = eReleaseBuffer;
507-
}
508-
/* Is the upper-layer checksum (TCP/UDP/ICMP) correct? */
509-
else if( usGenerateProtocolChecksum( ( uint8_t * ) ( pxNetworkBuffer->pucEthernetBuffer ), pxNetworkBuffer->xDataLength, pdFALSE ) != ipCORRECT_CRC )
510-
{
511-
/* Protocol checksum not accepted. */
512-
eReturn = eReleaseBuffer;
513-
}
514-
else
515-
{
516-
/* The checksum of the received packet is OK. */
517-
}
500+
/* Check sum in IP-header not correct. */
501+
eReturn = eReleaseBuffer;
502+
}
503+
/* Is the upper-layer checksum (TCP/UDP/ICMP) correct? */
504+
else if( usGenerateProtocolChecksum( ( uint8_t * ) ( pxNetworkBuffer->pucEthernetBuffer ), pxNetworkBuffer->xDataLength, pdFALSE ) != ipCORRECT_CRC )
505+
{
506+
/* Protocol checksum not accepted. */
507+
eReturn = eReleaseBuffer;
508+
}
509+
else
510+
{
511+
/* The checksum of the received packet is OK. */
518512
}
519513
}
520514
}

‎source/FreeRTOS_IPv6.c‎

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ const struct xIPv6_Address FreeRTOS_in6addr_loopback = { { 0U, 0U, 0U, 0U, 0U, 0
125125
/* Check if the complete IPv6-header plus protocol data have been transferred: */
126126
usPayloadLength = FreeRTOS_ntohs( pxIPv6Packet->xIPHeader.usPayloadLength );
127127

128-
if( uxBufferLength != ( size_t ) ( ipSIZE_OF_ETH_HEADER + ipSIZE_OF_IPv6_HEADER + ( size_t ) usPayloadLength ) )
128+
if( uxBufferLength < ( size_t ) ( ipSIZE_OF_ETH_HEADER + ipSIZE_OF_IPv6_HEADER + ( size_t ) usPayloadLength ) )
129129
{
130130
DEBUG_SET_TRACE_VARIABLE( xLocation, 4 );
131131
break;
@@ -542,22 +542,12 @@ eFrameProcessingResult_t prvAllowIPPacketIPv6( const IPHeader_IPv6_t * const pxI
542542
* define, so that the checksum won't be checked again here */
543543
if( eReturn == eProcessBuffer )
544544
{
545-
/* MISRA Ref 11.3.1 [Misaligned access] */
546-
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-113 */
547-
/* coverity[misra_c_2012_rule_11_3_violation] */
548-
const IPPacket_t * pxIPPacket = ( ( const IPPacket_t * ) pxNetworkBuffer->pucEthernetBuffer );
549-
const NetworkEndPoint_t * pxEndPoint = FreeRTOS_FindEndPointOnMAC( &( pxIPPacket->xEthernetHeader.xSourceAddress ), NULL );
550-
551545
/* IPv6 does not have a separate checksum in the IP-header */
552546
/* Is the upper-layer checksum (TCP/UDP/ICMP) correct? */
553-
/* Do not check the checksum of loop-back messages. */
554-
if( pxEndPoint == NULL )
547+
if( usGenerateProtocolChecksum( ( uint8_t * ) ( pxNetworkBuffer->pucEthernetBuffer ), pxNetworkBuffer->xDataLength, pdFALSE ) != ipCORRECT_CRC )
555548
{
556-
if( usGenerateProtocolChecksum( ( uint8_t * ) ( pxNetworkBuffer->pucEthernetBuffer ), pxNetworkBuffer->xDataLength, pdFALSE ) != ipCORRECT_CRC )
557-
{
558-
/* Protocol checksum not accepted. */
559-
eReturn = eReleaseBuffer;
560-
}
549+
/* Protocol checksum not accepted. */
550+
eReturn = eReleaseBuffer;
561551
}
562552
}
563553
}

‎test/unit-test/FreeRTOS_IPv4/FreeRTOS_IPv4_utest.c‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -623,7 +623,6 @@ void test_prvAllowIPPacketIPv4_EndpointDown_HappyPath( void )
623623
memcpy( pxIPPacket->xEthernetHeader.xDestinationAddress.ucBytes, xEndpoint.xMACAddress.ucBytes, sizeof( MACAddress_t ) );
624624

625625
FreeRTOS_IsEndPointUp_ExpectAndReturn( &xEndpoint, pdFALSE );
626-
FreeRTOS_FindEndPointOnMAC_ExpectAnyArgsAndReturn( NULL );
627626

628627
usGenerateChecksum_ExpectAndReturn( 0U, ( uint8_t * ) &( pxIPHeader->ucVersionHeaderLength ), ( size_t ) uxHeaderLength, ipCORRECT_CRC );
629628

@@ -704,7 +703,6 @@ void test_prvAllowIPPacketIPv4_EndpointDown_UnHappyPathBroadcast( void )
704703
memset( xEndpoint.xMACAddress.ucBytes, 0xCD, sizeof( MACAddress_t ) );
705704

706705
FreeRTOS_IsEndPointUp_ExpectAndReturn( &xEndpoint, pdFALSE );
707-
FreeRTOS_FindEndPointOnMAC_ExpectAnyArgsAndReturn( NULL );
708706

709707
usGenerateChecksum_ExpectAndReturn( 0U, ( uint8_t * ) &( pxIPHeader->ucVersionHeaderLength ), ( size_t ) uxHeaderLength, ipCORRECT_CRC );
710708

@@ -786,7 +784,6 @@ void test_prvAllowIPPacketIPv4_DestMACBrdCast_DestIPBroadcastAndIncorrectChkSum(
786784
memcpy( pxIPPacket->xEthernetHeader.xDestinationAddress.ucBytes, xBroadcastMACAddress.ucBytes, sizeof( MACAddress_t ) );
787785
FreeRTOS_IsEndPointUp_ExpectAndReturn( &xEndpoint, pdTRUE );
788786

789-
FreeRTOS_FindEndPointOnMAC_ExpectAnyArgsAndReturn( NULL );
790787

791788
usGenerateChecksum_ExpectAndReturn( 0U, ( uint8_t * ) &( pxIPHeader->ucVersionHeaderLength ), ( size_t ) uxHeaderLength, ipCORRECT_CRC - 1 );
792789

@@ -942,7 +939,6 @@ void test_prvAllowIPPacketIPv4_IncorrectChecksum( void )
942939
pxIPHeader->ulSourceIPAddress = 0xC0C00101;
943940

944941
FreeRTOS_IsEndPointUp_ExpectAndReturn( &xEndpoint, pdTRUE );
945-
FreeRTOS_FindEndPointOnMAC_ExpectAnyArgsAndReturn( NULL );
946942

947943
usGenerateChecksum_ExpectAndReturn( 0U, ( uint8_t * ) &( pxIPHeader->ucVersionHeaderLength ), ( size_t ) uxHeaderLength, ipCORRECT_CRC - 1 );
948944

@@ -986,7 +982,6 @@ void test_prvAllowIPPacketIPv4_IncorrectProtocolChecksum( void )
986982
pxIPHeader->ulSourceIPAddress = 0xC0C00101;
987983

988984
FreeRTOS_IsEndPointUp_ExpectAndReturn( &xEndpoint, pdTRUE );
989-
FreeRTOS_FindEndPointOnMAC_ExpectAnyArgsAndReturn( NULL );
990985

991986
usGenerateChecksum_ExpectAndReturn( 0U, ( uint8_t * ) &( pxIPHeader->ucVersionHeaderLength ), ( size_t ) uxHeaderLength, ipCORRECT_CRC );
992987

@@ -1031,7 +1026,6 @@ void test_prvAllowIPPacketIPv4_HappyPath( void )
10311026
pxIPHeader->ulSourceIPAddress = 0xC0C00101;
10321027

10331028
FreeRTOS_IsEndPointUp_ExpectAndReturn( &xEndpoint, pdTRUE );
1034-
FreeRTOS_FindEndPointOnMAC_ExpectAnyArgsAndReturn( NULL );
10351029

10361030
usGenerateChecksum_ExpectAndReturn( 0U, ( uint8_t * ) &( pxIPHeader->ucVersionHeaderLength ), ( size_t ) uxHeaderLength, ipCORRECT_CRC );
10371031

@@ -1073,7 +1067,8 @@ void test_prvAllowIPPacketIPv4_LoopbackHappyPath( void )
10731067

10741068
memcpy( pxIPPacket->xEthernetHeader.xDestinationAddress.ucBytes, xMACAddress.ucBytes, sizeof( MACAddress_t ) );
10751069

1076-
FreeRTOS_FindEndPointOnMAC_ExpectAnyArgsAndReturn( pxEndpoint );
1070+
usGenerateChecksum_ExpectAndReturn( 0U, ( uint8_t * ) &( pxIPHeader->ucVersionHeaderLength ), ( size_t ) uxHeaderLength, ipCORRECT_CRC );
1071+
usGenerateProtocolChecksum_ExpectAndReturn( ( uint8_t * ) ( pxNetworkBuffer->pucEthernetBuffer ), pxNetworkBuffer->xDataLength, pdFALSE, ipCORRECT_CRC );
10771072

10781073
eResult = prvAllowIPPacketIPv4( pxIPPacket, pxNetworkBuffer, uxHeaderLength );
10791074

@@ -1226,8 +1221,6 @@ static void xRunBadIPv4Loopback( uint32_t ulSource,
12261221

12271222
if( eExpected != eReleaseBuffer )
12281223
{
1229-
FreeRTOS_FindEndPointOnMAC_ExpectAnyArgsAndReturn( NULL );
1230-
12311224
usGenerateChecksum_ExpectAndReturn( 0U, ( uint8_t * ) &( pxIPHeader->ucVersionHeaderLength ), ( size_t ) uxHeaderLength, ipCORRECT_CRC );
12321225

12331226
usGenerateProtocolChecksum_ExpectAndReturn( ( uint8_t * ) ( pxNetworkBuffer->pucEthernetBuffer ), pxNetworkBuffer->xDataLength, pdFALSE, ipCORRECT_CRC );
@@ -1283,7 +1276,6 @@ void test_xBadIPv4Loopback_0_test( void )
12831276

12841277
FreeRTOS_IsEndPointUp_ExpectAndReturn( &xEndpoint, pdTRUE );
12851278

1286-
FreeRTOS_FindEndPointOnMAC_ExpectAnyArgsAndReturn( NULL );
12871279

12881280
usGenerateChecksum_ExpectAndReturn( 0U, ( uint8_t * ) &( pxIPHeader->ucVersionHeaderLength ), ( size_t ) uxHeaderLength, ipCORRECT_CRC );
12891281

‎test/unit-test/FreeRTOS_IPv6/FreeRTOS_IPv6_utest.c‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,6 @@ void test_prvAllowIPPacketIPv6_HappyPath()
104104

105105
pxTCPPacket->xIPHeader.ucVersionTrafficClass = 0x60U;
106106

107-
FreeRTOS_FindEndPointOnMAC_ExpectAndReturn( &pxTCPPacket->xEthernetHeader.xSourceAddress, NULL, NULL );
108107
usGenerateProtocolChecksum_ExpectAndReturn( pxNetworkBuffer->pucEthernetBuffer, pxNetworkBuffer->xDataLength, pdFALSE, ipCORRECT_CRC );
109108

110109
eResult = prvAllowIPPacketIPv6( &pxTCPPacket->xIPHeader, pxNetworkBuffer, 0U );
@@ -126,7 +125,6 @@ void test_prvAllowIPPacketIPv6_MulticastAddress()
126125
memcpy( pxTCPPacket->xIPHeader.xDestinationAddress.ucBytes, xMCIPAddress.ucBytes, ipSIZE_OF_IPv6_ADDRESS );
127126

128127
FreeRTOS_FindEndPointOnIP_IPv6_ExpectAndReturn( &( pxTCPPacket->xIPHeader.xSourceAddress ), pxNetworkBuffer->pxEndPoint );
129-
FreeRTOS_FindEndPointOnMAC_ExpectAndReturn( &pxTCPPacket->xEthernetHeader.xSourceAddress, NULL, NULL );
130128
usGenerateProtocolChecksum_ExpectAndReturn( pxNetworkBuffer->pucEthernetBuffer, pxNetworkBuffer->xDataLength, pdFALSE, ipCORRECT_CRC );
131129

132130
eResult = prvAllowIPPacketIPv6( &pxTCPPacket->xIPHeader, pxNetworkBuffer, 0U );
@@ -152,7 +150,6 @@ void test_prvAllowIPPacketIPv6_LoopbackAddress()
152150

153151
FreeRTOS_IsNetworkUp_IgnoreAndReturn( 0 );
154152

155-
FreeRTOS_FindEndPointOnMAC_ExpectAndReturn( &pxTCPPacket->xEthernetHeader.xSourceAddress, NULL, NULL );
156153
usGenerateProtocolChecksum_ExpectAndReturn( pxNetworkBuffer->pucEthernetBuffer, pxNetworkBuffer->xDataLength, pdFALSE, ipCORRECT_CRC );
157154

158155
eResult = prvAllowIPPacketIPv6( &pxTCPPacket->xIPHeader, pxNetworkBuffer, 0U );
@@ -174,7 +171,6 @@ void test_prvAllowIPPacketIPv6_LoopbackNotMatchDest()
174171

175172
FreeRTOS_FindEndPointOnIP_IPv6_ExpectAndReturn( &pxTCPPacket->xIPHeader.xSourceAddress, pxNetworkBuffer->pxEndPoint );
176173
FreeRTOS_IsNetworkUp_IgnoreAndReturn( 0 );
177-
FreeRTOS_FindEndPointOnMAC_ExpectAndReturn( &pxTCPPacket->xEthernetHeader.xSourceAddress, NULL, NULL );
178174
usGenerateProtocolChecksum_ExpectAndReturn( pxNetworkBuffer->pucEthernetBuffer, pxNetworkBuffer->xDataLength, pdFALSE, ipCORRECT_CRC );
179175

180176
eResult = prvAllowIPPacketIPv6( &pxTCPPacket->xIPHeader, pxNetworkBuffer, 0U );
@@ -218,7 +214,6 @@ void test_prvAllowIPPacketIPv6_NetworkDown()
218214

219215
FreeRTOS_FindEndPointOnIP_IPv6_ExpectAndReturn( &pxTCPPacket->xIPHeader.xSourceAddress, NULL );
220216
FreeRTOS_IsNetworkUp_IgnoreAndReturn( 0 );
221-
FreeRTOS_FindEndPointOnMAC_ExpectAndReturn( &pxTCPPacket->xEthernetHeader.xSourceAddress, NULL, NULL );
222217
usGenerateProtocolChecksum_ExpectAndReturn( pxNetworkBuffer->pucEthernetBuffer, pxNetworkBuffer->xDataLength, pdFALSE, ipCORRECT_CRC );
223218

224219
eResult = prvAllowIPPacketIPv6( &pxTCPPacket->xIPHeader, pxNetworkBuffer, 0U );
@@ -237,7 +232,7 @@ void test_prvAllowIPPacketIPv6_SelfSend()
237232

238233
pxTCPPacket->xIPHeader.ucVersionTrafficClass = 0x60U;
239234

240-
FreeRTOS_FindEndPointOnMAC_ExpectAndReturn( &pxTCPPacket->xEthernetHeader.xSourceAddress, NULL, pxNetworkBuffer->pxEndPoint );
235+
usGenerateProtocolChecksum_ExpectAndReturn( pxNetworkBuffer->pucEthernetBuffer, pxNetworkBuffer->xDataLength, pdFALSE, ipCORRECT_CRC );
241236

242237
eResult = prvAllowIPPacketIPv6( &pxTCPPacket->xIPHeader, pxNetworkBuffer, 0U );
243238
TEST_ASSERT_EQUAL( eProcessBuffer, eResult );
@@ -255,7 +250,6 @@ void test_prvAllowIPPacketIPv6_ChecksumError()
255250

256251
pxTCPPacket->xIPHeader.ucVersionTrafficClass = 0x60U;
257252

258-
FreeRTOS_FindEndPointOnMAC_ExpectAndReturn( &pxTCPPacket->xEthernetHeader.xSourceAddress, NULL, NULL );
259253
usGenerateProtocolChecksum_ExpectAndReturn( pxNetworkBuffer->pucEthernetBuffer, pxNetworkBuffer->xDataLength, pdFALSE, ipWRONG_CRC );
260254

261255
eResult = prvAllowIPPacketIPv6( &pxTCPPacket->xIPHeader, pxNetworkBuffer, 0U );

0 commit comments

Comments
 (0)