Fix potential out of bounds write (CWE-787) when processing LLMNR or mDNS queries (#1255)

tony-josi-aws · web-flow · commit 5f8526eb2958 · 2025-06-03T21:43:59.000+05:30
* Fix buffer overflow issue allowing out-of-bounds write when processing LLMNR or mDNS queries with very long DNS names when Buffer Allocation Scheme 1 is used

* [V4.3.2] Release updates

* Fix CI
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -129,7 +129,7 @@ jobs:
           exclude-dirs: source/portable/NetworkInterface/STM32
 
   formatting:
-    runs-on: ubuntu-20.04
+    runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4
       - name: Check formatting
diff --git a/History.txt b/History.txt
@@ -1,5 +1,15 @@
 Documentation and download available at https://www.FreeRTOS.org/
 
+Changes between FreeRTOS-plus-TCP V4.3.2 and V4.3.1 released June 03, 2025:
+	+ It was possible to cause an out-of-bounds write when processing LLMNR
+	  or mDNS queries with very long DNS names. This issue only affects systems
+	  using Buffer Allocation Scheme 1 with LLMNR or mDNS enabled.
+	  This issue has been fixed by adding checks to prevent out of bounds write.
+	  We would like to thank Paschal Amusuo (@AmPaschal),
+	  James C Davis (@davisjam), Taylor Le Lievre (@tlelievre26), and
+	  Aravind Kumar Machiry (@machiry) of Purdue University for collaborating
+	  on this issue through the coordinated vulnerability disclosure process.
+
 Changes between FreeRTOS-plus-TCP V4.3.1 and V4.3.0 released December 16, 2024:
 	+ Update README.md with information related to migrating to V4.3.0 and above
 	  for users utilising the STM32 network interface.
diff --git a/docs/doxygen/config.doxyfile b/docs/doxygen/config.doxyfile
@@ -48,7 +48,7 @@ PROJECT_NAME           = FreeRTOS-Plus-TCP
 # could be handy for archiving the generated documentation or if some version
 # control system is used.
 
-PROJECT_NUMBER         = V4.3.0
+PROJECT_NUMBER         = V4.3.2
 
 # Using the PROJECT_BRIEF tag one can provide an optional one line description
 # for a project that appears at the top of each page and should give viewer a
diff --git a/manifest.yml b/manifest.yml
@@ -1,5 +1,5 @@
 name: "FreeRTOS-Plus-TCP"
-version: "V4.3.1"
+version: "V4.3.2"
 description:
   "Thread safe FreeRTOS TCP/IP stack working on top of the FreeRTOS-Kernel to
   implement the TCP/IP protocol. Suitable for microcontrollers."
diff --git a/source/FreeRTOS_DNS_Parser.c b/source/FreeRTOS_DNS_Parser.c
@@ -486,31 +486,30 @@
                             LLMNRAnswer_t * pxAnswer;
                             uint8_t * pucNewBuffer = NULL;
                             size_t uxExtraLength;
+                            size_t uxDataLength = uxBufferLength +
+                                                  sizeof( UDPHeader_t ) +
+                                                  sizeof( EthernetHeader_t ) +
+                                                  uxIPHeaderSizePacket( pxNetworkBuffer );
 
-                            if( xBufferAllocFixedSize == pdFALSE )
-                            {
-                                size_t uxDataLength = uxBufferLength +
-                                                      sizeof( UDPHeader_t ) +
-                                                      sizeof( EthernetHeader_t ) +
-                                                      uxIPHeaderSizePacket( pxNetworkBuffer );
-
-                                #if ( ipconfigUSE_IPv6 != 0 )
-                                    if( xSet.usType == dnsTYPE_AAAA_HOST )
-                                    {
-                                        uxExtraLength = sizeof( LLMNRAnswer_t ) + ipSIZE_OF_IPv6_ADDRESS - sizeof( pxAnswer->ulIPAddress );
-                                    }
-                                    else
-                                #endif /* ( ipconfigUSE_IPv6 != 0 ) */
-                                #if ( ipconfigUSE_IPv4 != 0 )
-                                {
-                                    uxExtraLength = sizeof( LLMNRAnswer_t );
-                                }
-                                #else /* ( ipconfigUSE_IPv4 != 0 ) */
+                            #if ( ipconfigUSE_IPv6 != 0 )
+                                if( xSet.usType == dnsTYPE_AAAA_HOST )
                                 {
-                                    /* do nothing, coverity happy */
+                                    uxExtraLength = sizeof( LLMNRAnswer_t ) + ipSIZE_OF_IPv6_ADDRESS - sizeof( pxAnswer->ulIPAddress );
                                 }
-                                #endif /* ( ipconfigUSE_IPv4 != 0 ) */
+                                else
+                            #endif /* ( ipconfigUSE_IPv6 != 0 ) */
+                            #if ( ipconfigUSE_IPv4 != 0 )
+                            {
+                                uxExtraLength = sizeof( LLMNRAnswer_t );
+                            }
+                            #else /* ( ipconfigUSE_IPv4 != 0 ) */
+                            {
+                                /* do nothing, coverity happy */
+                            }
+                            #endif /* ( ipconfigUSE_IPv4 != 0 ) */
 
+                            if( xBufferAllocFixedSize == pdFALSE )
+                            {
                                 /* Set the size of the outgoing packet. */
                                 pxNetworkBuffer->xDataLength = uxDataLength;
                                 pxNewBuffer = pxDuplicateNetworkBufferWithDescriptor( pxNetworkBuffer,
@@ -539,7 +538,17 @@
                             }
                             else
                             {
-                                pucNewBuffer = &( pxNetworkBuffer->pucEthernetBuffer[ uxUDPOffset ] );
+                                /* When xBufferAllocFixedSize is TRUE, check if the buffer size is big enough to
+                                 * store the answer. */
+                                if( ( uxDataLength + uxExtraLength ) <= ipconfigNETWORK_MTU + ipSIZE_OF_ETH_HEADER )
+                                {
+                                    pucNewBuffer = &( pxNetworkBuffer->pucEthernetBuffer[ uxUDPOffset ] );
+                                }
+                                else
+                                {
+                                    /* Just to indicate that the message may not be answered. */
+                                    pxNetworkBuffer = NULL;
+                                }
                             }
 
                             if( ( pxNetworkBuffer != NULL ) )
@@ -1210,7 +1219,11 @@
                 {
                     /* BufferAllocation_1.c is used, the Network Buffers can contain at least
                      * ipconfigNETWORK_MTU + ipSIZE_OF_ETH_HEADER. */
-                    configASSERT( uxSizeNeeded < ipconfigNETWORK_MTU + ipSIZE_OF_ETH_HEADER );
+                    if( uxSizeNeeded > ( ipconfigNETWORK_MTU + ipSIZE_OF_ETH_HEADER ) )
+                    {
+                        /* The buffer is too small to reply. Drop silently. */
+                        break;
+                    }
                 }
 
                 pxNetworkBuffer->xDataLength = uxSizeNeeded;
diff --git a/source/portable/BufferManagement/BufferAllocation_1.c b/source/portable/BufferManagement/BufferAllocation_1.c
@@ -237,11 +237,8 @@ NetworkBufferDescriptor_t * pxGetNetworkBufferWithDescriptor( size_t xRequestedS
     BaseType_t xInvalid = pdFALSE;
     UBaseType_t uxCount;
 
-    /* The current implementation only has a single size memory block, so
-     * the requested size parameter is not used (yet). */
-    ( void ) xRequestedSizeBytes;
-
-    if( xNetworkBufferSemaphore != NULL )
+    if( ( xNetworkBufferSemaphore != NULL ) &&
+        ( xRequestedSizeBytes <= ( ipconfigNETWORK_MTU + ipSIZE_OF_ETH_HEADER ) ) )
     {
         /* If there is a semaphore available, there is a network buffer
          * available. */
@@ -432,10 +429,18 @@ UBaseType_t uxGetNumberOfFreeNetworkBuffers( void )
 NetworkBufferDescriptor_t * pxResizeNetworkBufferWithDescriptor( NetworkBufferDescriptor_t * pxNetworkBuffer,
                                                                  size_t xNewSizeBytes )
 {
-    /* In BufferAllocation_1.c all network buffer are allocated with a
-     * maximum size of 'ipTOTAL_ETHERNET_FRAME_SIZE'.No need to resize the
-     * network buffer. */
-    pxNetworkBuffer->xDataLength = xNewSizeBytes;
+    if( xNewSizeBytes <= ( ipconfigNETWORK_MTU + ipSIZE_OF_ETH_HEADER ) )
+    {
+        /* In BufferAllocation_1.c all network buffer are allocated with a
+         * maximum size of 'ipTOTAL_ETHERNET_FRAME_SIZE'.No need to resize the
+         * network buffer. */
+        pxNetworkBuffer->xDataLength = xNewSizeBytes;
+    }
+    else
+    {
+        pxNetworkBuffer = NULL;
+    }
+
     return pxNetworkBuffer;
 }
 
diff --git a/test/cbmc/proofs/DNS/DNSTreatNBNS/DNS_TreatNBNS_harness.c b/test/cbmc/proofs/DNS/DNSTreatNBNS/DNS_TreatNBNS_harness.c
@@ -122,10 +122,7 @@ void harness()
 
     BaseType_t xDataSize;
 
-    /* When re-adjusting the buffer, (sizeof( NBNSAnswer_t ) - 2 * sizeof( uint16_t )) more bytes are
-     * required to be added to the existing buffer. Make sure total bytes doesn't exceed  ipconfigNETWORK_MTU + ipSIZE_OF_ETH_HEADER
-     * when re-resizing. This will prevent hitting an assert if Buffer Allocation 1 is used. */
-    __CPROVER_assume( ( xDataSize != 0 ) && ( xDataSize < ( ipconfigNETWORK_MTU + ipSIZE_OF_ETH_HEADER - ( sizeof( NBNSAnswer_t ) - 2 * sizeof( uint16_t ) ) ) ) );
+    __CPROVER_assume( ( xDataSize > 0 ) && ( xDataSize < ( ipconfigNETWORK_MTU + ipSIZE_OF_ETH_HEADER ) ) );
 
     xNetworkBuffer.pucEthernetBuffer = safeMalloc( xDataSize );
     xNetworkBuffer.xDataLength = xDataSize;
diff --git a/test/cbmc/proofs/DNS/DNSTreatNBNS/Makefile.json b/test/cbmc/proofs/DNS/DNSTreatNBNS/Makefile.json
@@ -23,6 +23,7 @@
   "DEF":
   [
     "ipconfigUSE_DNS_CACHE={USE_CACHE}",
-    "ipconfigUSE_NBNS=1"
+    "ipconfigUSE_NBNS=1",
+    "ipconfigNETWORK_MTU=586"
   ]
 }
diff --git a/test/cbmc/proofs/DNS_ParseDNSReply/Configurations.json b/test/cbmc/proofs/DNS_ParseDNSReply/Configurations.json
@@ -1,16 +1,18 @@
 {
   "ENTRY": "DNS_ParseDNSReply",
-  "TEST_PAYLOAD_SIZE": 2,
-  "TEST_IPV4_PACKET_SIZE": 29,
-  "TEST_IPV6_PACKET_SIZE": 49,
+  "TEST_MAX_TEST_UNWIND_LOOP": 6,
+  "TEST_MIN_TEST_DNS_HEADER": 12,
+  "TEST_MIN_IPV4_UDP_PACKET_SIZE": 42,
+  "TEST_MIN_IPV6_UDP_PACKET_SIZE": 62,
+  "TEST_IPV4_NETWORK_MTU": "__eval {TEST_MIN_IPV4_UDP_PACKET_SIZE} + {TEST_MIN_TEST_DNS_HEADER} + {TEST_MAX_TEST_UNWIND_LOOP}",
+  "TEST_IPV6_NETWORK_MTU": "__eval {TEST_MIN_IPV6_UDP_PACKET_SIZE} + {TEST_MIN_TEST_DNS_HEADER} + {TEST_MAX_TEST_UNWIND_LOOP}",
   "CBMCFLAGS":
   [
     "--unwind 1",
-    "--unwindset DNS_ParseDNSReply.0:{TEST_PAYLOAD_SIZE}",
-    "--unwindset DNS_ReadNameField.0:{TEST_PAYLOAD_SIZE}",
-    "--unwindset DNS_ReadNameField.1:{TEST_PAYLOAD_SIZE}",
-    "--unwindset parseDNSAnswer.0:{TEST_PAYLOAD_SIZE}",
-    "--unwindset strncpy.0:{TEST_PAYLOAD_SIZE}"
+    "--unwindset strlen.0:{TEST_MAX_TEST_UNWIND_LOOP}",
+    "--unwindset DNS_ParseDNSReply.0:{TEST_MAX_TEST_UNWIND_LOOP}",
+    "--unwindset DNS_ReadNameField.0:{TEST_MAX_TEST_UNWIND_LOOP}",
+    "--unwindset DNS_ReadNameField.1:{TEST_MAX_TEST_UNWIND_LOOP}"
   ],
   "OPT":
   [
@@ -25,21 +27,63 @@
   "DEF":
   [
     {
-      "IPv4":
+      "IPv4_FixedNetworkBufferSize":
       [
-        "TEST_PACKET_SIZE={TEST_IPV4_PACKET_SIZE}",
+        "TEST_MAX_PAYLOAD_SIZE={TEST_MAX_TEST_UNWIND_LOOP}",
         "ipconfigUSE_LLMNR=1",
         "ipconfigUSE_MDNS=1",
-        "IS_TESTING_IPV6=0"
+        "IS_TESTING_IPV6=0",
+        "IS_BUFFER_ALLOCATE_FIXED=1",
+        "ipconfigNETWORK_MTU={TEST_IPV4_NETWORK_MTU}",
+        "ipconfigUSE_TCP=0",
+        "ipconfigUSE_DHCP=0",
+        "ipconfigTCP_MSS=536",
+        "ipconfigDNS_CACHE_NAME_LENGTH={TEST_MAX_TEST_UNWIND_LOOP}"
       ]
     },
     {
-      "IPv6":
+      "IPv6_FixedNetworkBufferSize":
       [
-        "TEST_PACKET_SIZE={TEST_IPV6_PACKET_SIZE}",
+        "TEST_MAX_PAYLOAD_SIZE={TEST_MAX_TEST_UNWIND_LOOP}",
         "ipconfigUSE_LLMNR=1",
         "ipconfigUSE_MDNS=1",
-        "IS_TESTING_IPV6=1"
+        "IS_TESTING_IPV6=1",
+        "IS_BUFFER_ALLOCATE_FIXED=1",
+        "ipconfigNETWORK_MTU={TEST_IPV6_NETWORK_MTU}",
+        "ipconfigUSE_TCP=0",
+        "ipconfigUSE_DHCP=0",
+        "ipconfigTCP_MSS=536",
+        "ipconfigDNS_CACHE_NAME_LENGTH={TEST_MAX_TEST_UNWIND_LOOP}"
+      ]
+    },
+    {
+      "IPv4_DynamicNetworkBufferSize":
+      [
+        "TEST_MAX_PAYLOAD_SIZE={TEST_MAX_TEST_UNWIND_LOOP}",
+        "ipconfigUSE_LLMNR=1",
+        "ipconfigUSE_MDNS=1",
+        "IS_TESTING_IPV6=0",
+        "IS_BUFFER_ALLOCATE_FIXED=0",
+        "ipconfigNETWORK_MTU={TEST_IPV4_NETWORK_MTU}",
+        "ipconfigUSE_TCP=0",
+        "ipconfigUSE_DHCP=0",
+        "ipconfigTCP_MSS=536",
+        "ipconfigDNS_CACHE_NAME_LENGTH={TEST_MAX_TEST_UNWIND_LOOP}"
+      ]
+    },
+    {
+      "IPv6_DynamicNetworkBufferSize":
+      [
+        "TEST_MAX_PAYLOAD_SIZE={TEST_MAX_TEST_UNWIND_LOOP}",
+        "ipconfigUSE_LLMNR=1",
+        "ipconfigUSE_MDNS=1",
+        "IS_TESTING_IPV6=1",
+        "IS_BUFFER_ALLOCATE_FIXED=0",
+        "ipconfigNETWORK_MTU={TEST_IPV6_NETWORK_MTU}",
+        "ipconfigUSE_TCP=0",
+        "ipconfigUSE_DHCP=0",
+        "ipconfigTCP_MSS=536",
+        "ipconfigDNS_CACHE_NAME_LENGTH={TEST_MAX_TEST_UNWIND_LOOP}"
       ]
     }
   ],
diff --git a/test/cbmc/proofs/DNS_ParseDNSReply/DNS_ParseDNSReply_harness.c b/test/cbmc/proofs/DNS_ParseDNSReply/DNS_ParseDNSReply_harness.c
@@ -10,14 +10,16 @@
 
 /* FreeRTOS+TCP includes. */
 #include "FreeRTOS_IP.h"
+#include "FreeRTOS_IP_Private.h"
 #include "FreeRTOS_DNS.h"
 #include "FreeRTOS_DNS_Parser.h"
 #include "NetworkBufferManagement.h"
 #include "NetworkInterface.h"
 #include "IPTraceMacroDefaults.h"
 
 #include "cbmc.h"
-#include "../../utility/memory_assignments.c"
+
+const BaseType_t xBufferAllocFixedSize = IS_BUFFER_ALLOCATE_FIXED;
 
 /****************************************************************
 * Signature of function under test
@@ -60,12 +62,18 @@ NetworkBufferDescriptor_t * pxUDPPayloadBuffer_to_NetworkBuffer( const void * pv
 
 uint32_t ulChar2u32( const uint8_t * pucPtr )
 {
+    uint32_t ret;
+
     __CPROVER_assert( __CPROVER_r_ok( pucPtr, 4 ), "must be 4 bytes legal address to read" );
+    return ret;
 }
 
 uint16_t usChar2u16( const uint8_t * pucPtr )
 {
+    uint16_t ret;
+
     __CPROVER_assert( __CPROVER_r_ok( pucPtr, 2 ), "must be 2 bytes legal address to read" );
+    return ret;
 }
 
 const char * FreeRTOS_inet_ntop( BaseType_t xAddressFamily,
@@ -131,11 +139,14 @@ NetworkBufferDescriptor_t * pxDuplicateNetworkBufferWithDescriptor( const Networ
 {
     NetworkBufferDescriptor_t * pxNetworkBuffer = safeMalloc( sizeof( NetworkBufferDescriptor_t ) );
 
-    if( ensure_memory_is_valid( pxNetworkBuffer, xNewLength ) )
+    if( pxNetworkBuffer != NULL )
     {
         pxNetworkBuffer->pucEthernetBuffer = safeMalloc( xNewLength );
-        __CPROVER_assume( pxNetworkBuffer->pucEthernetBuffer );
+        __CPROVER_assume( pxNetworkBuffer->pucEthernetBuffer != NULL );
         pxNetworkBuffer->xDataLength = xNewLength;
+
+        pxNetworkBuffer->pxEndPoint = safeMalloc( sizeof( NetworkEndPoint_t ) );
+        __CPROVER_assume( pxNetworkBuffer->pxEndPoint != NULL );
     }
 
     return pxNetworkBuffer;
@@ -176,23 +187,31 @@ void harness()
     uint8_t * pPayloadBuffer;
     size_t uxPayloadBufferLength;
 
-    __CPROVER_assert( TEST_PACKET_SIZE < CBMC_MAX_OBJECT_SIZE,
-                      "TEST_PACKET_SIZE < CBMC_MAX_OBJECT_SIZE" );
-
-    __CPROVER_assume( uxBufferLength < CBMC_MAX_OBJECT_SIZE );
-    __CPROVER_assume( uxBufferLength <= TEST_PACKET_SIZE );
+    __CPROVER_assume( uxBufferLength <= ipconfigNETWORK_MTU );
+    __CPROVER_assume( pxNetworkEndPoint_Temp != NULL );
 
     lIsIPv6Packet = IS_TESTING_IPV6;
 
-    xNetworkBuffer.pucEthernetBuffer = safeMalloc( uxBufferLength );
-    xNetworkBuffer.xDataLength = uxBufferLength;
-    xNetworkBuffer.pxEndPoint = pxNetworkEndPoint_Temp;
+    if( xBufferAllocFixedSize != pdFALSE )
+    {
+        /* When xBufferAllocFixedSize is true, buffers in all network descriptors
+         * is big enough to allow all Ethernet packet. */
+        xNetworkBuffer.pucEthernetBuffer = safeMalloc( ipconfigNETWORK_MTU + ipSIZE_OF_ETH_HEADER );
+        xNetworkBuffer.xDataLength = uxBufferLength;
+        xNetworkBuffer.pxEndPoint = pxNetworkEndPoint_Temp;
+    }
+    else
+    {
+        xNetworkBuffer.pucEthernetBuffer = safeMalloc( uxBufferLength );
+        xNetworkBuffer.xDataLength = uxBufferLength;
+        xNetworkBuffer.pxEndPoint = pxNetworkEndPoint_Temp;
+    }
 
     __CPROVER_assume( xNetworkBuffer.pucEthernetBuffer != NULL );
 
     if( lIsIPv6Packet )
     {
-        __CPROVER_assume( uxBufferLength >= ulIpv6UdpOffset ); /* 62 is total size of IPv4 UDP header, including ethernet, IPv6, UDP headers. */
+        __CPROVER_assume( uxBufferLength >= ulIpv6UdpOffset ); /* 62 is total size of IPv6 UDP header, including ethernet, IPv6, UDP headers. */
         pPayloadBuffer = xNetworkBuffer.pucEthernetBuffer + ulIpv6UdpOffset;
         uxPayloadBufferLength = uxBufferLength - ulIpv6UdpOffset;
     }
diff --git a/test/cbmc/proofs/parsing/ProcessReceivedTCPPacket/ProcessReceivedTCPPacket_harness.c b/test/cbmc/proofs/parsing/ProcessReceivedTCPPacket/ProcessReceivedTCPPacket_harness.c
diff --git a/test/cbmc/proofs/parsing/ProcessReceivedTCPPacket_IPv6/ProcessReceivedTCPPacket_IPv6_harness.c b/test/cbmc/proofs/parsing/ProcessReceivedTCPPacket_IPv6/ProcessReceivedTCPPacket_IPv6_harness.c
diff --git a/test/unit-test/FreeRTOS_DNS_Parser/FreeRTOS_DNS_Parser_utest.c b/test/unit-test/FreeRTOS_DNS_Parser/FreeRTOS_DNS_Parser_utest.c

Original file line number	Diff line number	Diff line change
`@@ -23,6 +23,7 @@`
`23`	`23`	`"DEF":`
`24`	`24`	`[`
`25`	`25`	`"ipconfigUSE_DNS_CACHE={USE_CACHE}",`
`26`		`- "ipconfigUSE_NBNS=1"`
	`26`	`+ "ipconfigUSE_NBNS=1",`
	`27`	`+ "ipconfigNETWORK_MTU=586"`
`27`	`28`	`]`
`28`	`29`	`}`