CVE-2021-32020: Improper Restriction of Operations within the Bounds of a Memory Buffer (CWE-119)
value: The kernel in Amazon Web Services FreeRTOS before 10.4.3 has insufficient bounds checking during management of heap memory.
CWE description: The product performs operations on a memory buffer, but it reads from or writes to a memory location outside the buffer's intended boundary. This may result in read or write operations on unexpected memory locations that could be linked to other variables, data structures, or internal program data.
CVSS v3 base score: 9.8
Published: 2021-05-03T22:15Z
Repository: https://github.com/FreeRTOS/FreeRTOS-Kernel
Language: C
Files changed: portable/MemMang/heap_1.c, portable/MemMang/heap_2.c, portable/MemMang/heap_4.c, portable/MemMang/heap_5.c
Commit date: 2020-12-07 10:36:27 -0800
Commit message: Improve heap2 bounds checking (#224)

* Improve heap bounds checking in pvPortMalloc
Diff: diff --git a/portable/MemMang/heap_1.c b/portable/MemMang/heap_1.c
index 4a7f8c2e5..2b7346340 100644
--- a/portable/MemMang/heap_1.c
+++ b/portable/MemMang/heap_1.c
@@ -20,11 +20,10 @@
  * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
  *
  * https://www.FreeRTOS.org
  * https://github.com/FreeRTOS
  *
- * 1 tab == 4 spaces!
  */
 
 
 /*
  * The simplest possible implementation of pvPortMalloc().  Note that this
@@ -70,17 +69,24 @@ static size_t xNextFreeByte = ( size_t ) 0;
 void * pvPortMalloc( size_t xWantedSize )
 {
     void * pvReturn = NULL;
     static uint8_t * pucAlignedHeap = NULL;
 
-    /* Ensure that blocks are always aligned to the required number of bytes. */
+    /* Ensure that blocks are always aligned. */
     #if ( portBYTE_ALIGNMENT != 1 )
         {
             if( xWantedSize & portBYTE_ALIGNMENT_MASK )
             {
-                /* Byte alignment required. */
-                xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) );
+                /* Byte alignment required. Check for overflow. */
+                if ( (xWantedSize + ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) )) > xWantedSize )
+                {
+                    xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) );
+                } 
+                else 
+                {
+                    xWantedSize = 0;
+                }
             }
         }
     #endif
 
     vTaskSuspendAll();
@@ -89,12 +95,13 @@ void * pvPortMalloc( size_t xWantedSize )
         {
             /* Ensure the heap starts on a correctly aligned boundary. */
             pucAlignedHeap = ( uint8_t * ) ( ( ( portPOINTER_SIZE_TYPE ) & ucHeap[ portBYTE_ALIGNMENT ] ) & ( ~( ( portPOINTER_SIZE_TYPE ) portBYTE_ALIGNMENT_MASK ) ) );
         }
 
-        /* Check there is enough room left for the allocation. */
-        if( ( ( xNextFreeByte + xWantedSize ) < configADJUSTED_HEAP_SIZE ) &&
+        /* Check there is enough room left for the allocation and. */
+        if( ( xWantedSize > 0 ) && /* valid size */
+            ( ( xNextFreeByte + xWantedSize ) < configADJUSTED_HEAP_SIZE ) &&
             ( ( xNextFreeByte + xWantedSize ) > xNextFreeByte ) ) /* Check for overflow. */
         {
             /* Return the next free byte then increment the index past this
              * block. */
             pvReturn = pucAlignedHeap + xNextFreeByte;
diff --git a/portable/MemMang/heap_2.c b/portable/MemMang/heap_2.c
index 640cd54fd..e132ae3ea 100644
--- a/portable/MemMang/heap_2.c
+++ b/portable/MemMang/heap_2.c
@@ -20,11 +20,10 @@
  * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
  *
  * https://www.FreeRTOS.org
  * https://github.com/FreeRTOS
  *
- * 1 tab == 4 spaces!
  */
 
 /*
  * A sample implementation of pvPortMalloc() and vPortFree() that permits
  * allocated blocks to be freed, but does not combine adjacent free blocks
@@ -130,25 +129,36 @@ void * pvPortMalloc( size_t xWantedSize )
         {
             prvHeapInit();
             xHeapHasBeenInitialised = pdTRUE;
         }
 
-        /* The wanted size is increased so it can contain a BlockLink_t
+        /* The wanted size must be increased so it can contain a BlockLink_t
          * structure in addition to the requested amount of bytes. */
-        if( xWantedSize > 0 )
+        if( ( xWantedSize > 0 ) && 
+            ( ( xWantedSize + heapSTRUCT_SIZE ) >  xWantedSize ) ) /* Overflow check */
         {
             xWantedSize += heapSTRUCT_SIZE;
 
-            /* Ensure that blocks are always aligned to the required number of bytes. */
-            if( ( xWantedSize & portBYTE_ALIGNMENT_MASK ) != 0 )
+            /* Byte alignment required. Check for overflow. */
+            if( ( xWantedSize + ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) ) ) 
+                    > xWantedSize )
             {
-                /* Byte alignment required. */
                 xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) );
+                configASSERT( ( xWantedSize & portBYTE_ALIGNMENT_MASK ) == 0 );
             }
+            else
+            {
+                xWantedSize = 0;
+            }       
+        }
+        else 
+        {
+            xWantedSize = 0; 
         }
 
-        if( ( xWantedSize > 0 ) && ( xWantedSize < configADJUSTED_HEAP_SIZE ) )
+
+        if( ( xWantedSize > 0 ) && ( xWantedSize <= xFreeBytesRemaining ) )
         {
             /* Blocks are stored in byte order - traverse the list from the start
              * (smallest) block until one of adequate size is found. */
             pxPreviousBlock = &xStart;
             pxBlock = xStart.pxNextFreeBlock;
diff --git a/portable/MemMang/heap_4.c b/portable/MemMang/heap_4.c
index 7e5357ccf..2a1ee20ff 100644
--- a/portable/MemMang/heap_4.c
+++ b/portable/MemMang/heap_4.c
@@ -134,49 +134,57 @@ void * pvPortMalloc( size_t xWantedSize )
          * set.  The top bit of the block size member of the BlockLink_t structure
          * is used to determine who owns the block - the application or the
          * kernel, so it must be free. */
         if( ( xWantedSize & xBlockAllocatedBit ) == 0 )
         {
-            /* The wanted size is increased so it can contain a BlockLink_t
+            /* The wanted size must be increased so it can contain a BlockLink_t
              * structure in addition to the requested amount of bytes. */
-            if( xWantedSize > 0 )
+            if( ( xWantedSize > 0 ) && 
+                ( ( xWantedSize + xHeapStructSize ) >  xWantedSize ) ) /* Overflow check */
             {
                 xWantedSize += xHeapStructSize;
 
-                /* Ensure that blocks are always aligned to the required number
-                 * of bytes. */
+                /* Ensure that blocks are always aligned. */
                 if( ( xWantedSize & portBYTE_ALIGNMENT_MASK ) != 0x00 )
                 {
-                    /* Byte alignment required. */
-                    xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) );
-                    configASSERT( ( xWantedSize & portBYTE_ALIGNMENT_MASK ) == 0 );
+                    /* Byte alignment required. Check for overflow. */
+                    if( ( xWantedSize + ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) ) ) 
+                            > xWantedSize )
+                    {
+                        xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) );
+                        configASSERT( ( xWantedSize & portBYTE_ALIGNMENT_MASK ) == 0 );
+                    }
+                    else
+                    {
+                        xWantedSize = 0;
+                    }  
                 }
                 else
                 {
                     mtCOVERAGE_TEST_MARKER();
                 }
-            }
-            else
+            } 
+            else 
             {
-                mtCOVERAGE_TEST_MARKER();
+                xWantedSize = 0;
             }
 
             if( ( xWantedSize > 0 ) && ( xWantedSize <= xFreeBytesRemaining ) )
             {
                 /* Traverse the list from the start	(lowest address) block until
-                 * one	of adequate size is found. */
+                 * one of adequate size is found. */
                 pxPreviousBlock = &xStart;
                 pxBlock = xStart.pxNextFreeBlock;
 
                 while( ( pxBlock->xBlockSize < xWantedSize ) && ( pxBlock->pxNextFreeBlock != NULL ) )
                 {
                     pxPreviousBlock = pxBlock;
                     pxBlock = pxBlock->pxNextFreeBlock;
                 }
 
                 /* If the end marker was reached then a block of adequate size
-                 * was	not found. */
+                 * was not found. */
                 if( pxBlock != pxEnd )
                 {
                     /* Return the memory space pointed to - jumping over the
                      * BlockLink_t structure at its start. */
                     pvReturn = ( void * ) ( ( ( uint8_t * ) pxPreviousBlock->pxNextFreeBlock ) + xHeapStructSize );
diff --git a/portable/MemMang/heap_5.c b/portable/MemMang/heap_5.c
index 90c0b5bdf..fe194a6a0 100644
--- a/portable/MemMang/heap_5.c
+++ b/portable/MemMang/heap_5.c
@@ -20,11 +20,10 @@
  * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
  *
  * https://www.FreeRTOS.org
  * https://github.com/FreeRTOS
  *
- * 1 tab == 4 spaces!
  */
 
 /*
  * A sample implementation of pvPortMalloc() that allows the heap to be defined
  * across multiple non-contigous blocks and combines (coalescences) adjacent
@@ -148,46 +147,54 @@ void * pvPortMalloc( size_t xWantedSize )
          * kernel, so it must be free. */
         if( ( xWantedSize & xBlockAllocatedBit ) == 0 )
         {
             /* The wanted size is increased so it can contain a BlockLink_t
              * structure in addition to the requested amount of bytes. */
-            if( xWantedSize > 0 )
+            if( ( xWantedSize > 0 ) && 
+                ( ( xWantedSize + xHeapStructSize ) >  xWantedSize ) ) /* Overflow check */
             {
                 xWantedSize += xHeapStructSize;
 
-                /* Ensure that blocks are always aligned to the required number
-                 * of bytes. */
+                /* Ensure that blocks are always aligned */
                 if( ( xWantedSize & portBYTE_ALIGNMENT_MASK ) != 0x00 )
                 {
-                    /* Byte alignment required. */
-                    xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) );
+                    /* Byte alignment required. Check for overflow */
+                    if( ( xWantedSize + ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) ) ) >
+                         xWantedSize )
+                    {
+                        xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) );
+                    } 
+                    else 
+                    {
+                        xWantedSize = 0;
+                    }
                 }
                 else
                 {
                     mtCOVERAGE_TEST_MARKER();
                 }
             }
             else
             {
-                mtCOVERAGE_TEST_MARKER();
+                xWantedSize = 0;
             }
 
             if( ( xWantedSize > 0 ) && ( xWantedSize <= xFreeBytesRemaining ) )
             {
                 /* Traverse the list from the start	(lowest address) block until
-                 * one	of adequate size is found. */
+                 * one of adequate size is found. */
                 pxPreviousBlock = &xStart;
                 pxBlock = xStart.pxNextFreeBlock;
 
                 while( ( pxBlock->xBlockSize < xWantedSize ) && ( pxBlock->pxNextFreeBlock != NULL ) )
                 {
                     pxPreviousBlock = pxBlock;
                     pxBlock = pxBlock->pxNextFreeBlock;
                 }
 
                 /* If the end marker was reached then a block of adequate size
-                 * was	not found. */
+                 * was not found. */
                 if( pxBlock != pxEnd )
                 {
                     /* Return the memory space pointed to - jumping over the
                      * BlockLink_t structure at its start. */
                     pvReturn = ( void * ) ( ( ( uint8_t * ) pxPreviousBlock->pxNextFreeBlock ) + xHeapStructSize );

Vulnerable code:  * 1 tab == 4 spaces!
    /* Ensure that blocks are always aligned to the required number of bytes. */
                /* Byte alignment required. */
                xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) );
        /* Check there is enough room left for the allocation. */
        if( ( ( xNextFreeByte + xWantedSize ) < configADJUSTED_HEAP_SIZE ) &&
 * 1 tab == 4 spaces!
        /* The wanted size is increased so it can contain a BlockLink_t
        if( xWantedSize > 0 )
            /* Ensure that blocks are always aligned to the required number of bytes. */
            if( ( xWantedSize & portBYTE_ALIGNMENT_MASK ) != 0 )
                /* Byte alignment required. */
        if( ( xWantedSize > 0 ) && ( xWantedSize < configADJUSTED_HEAP_SIZE ) )
            /* The wanted size is increased so it can contain a BlockLink_t
            if( xWantedSize > 0 )
                /* Ensure that blocks are always aligned to the required number
                 * of bytes. */
                    /* Byte alignment required. */
                    xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) );
                    configASSERT( ( xWantedSize & portBYTE_ALIGNMENT_MASK ) == 0 );
            }
            else
                mtCOVERAGE_TEST_MARKER();
                 * one	of adequate size is found. */
                 * was	not found. */
 * 1 tab == 4 spaces!
            if( xWantedSize > 0 )
                /* Ensure that blocks are always aligned to the required number
                 * of bytes. */
                    /* Byte alignment required. */
                    xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) );
                mtCOVERAGE_TEST_MARKER();
                 * one	of adequate size is found. */
                 * was	not found. */
Fixed code:     /* Ensure that blocks are always aligned. */
                /* Byte alignment required. Check for overflow. */
                if ( (xWantedSize + ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) )) > xWantedSize )
                {
                    xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) );
                } 
                else 
                {
                    xWantedSize = 0;
                }
        /* Check there is enough room left for the allocation and. */
        if( ( xWantedSize > 0 ) && /* valid size */
            ( ( xNextFreeByte + xWantedSize ) < configADJUSTED_HEAP_SIZE ) &&
        /* The wanted size must be increased so it can contain a BlockLink_t
        if( ( xWantedSize > 0 ) && 
            ( ( xWantedSize + heapSTRUCT_SIZE ) >  xWantedSize ) ) /* Overflow check */
            /* Byte alignment required. Check for overflow. */
            if( ( xWantedSize + ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) ) ) 
                    > xWantedSize )
                configASSERT( ( xWantedSize & portBYTE_ALIGNMENT_MASK ) == 0 );
            else
            {
                xWantedSize = 0;
            }       
        }
        else 
        {
            xWantedSize = 0; 

        if( ( xWantedSize > 0 ) && ( xWantedSize <= xFreeBytesRemaining ) )
            /* The wanted size must be increased so it can contain a BlockLink_t
            if( ( xWantedSize > 0 ) && 
                ( ( xWantedSize + xHeapStructSize ) >  xWantedSize ) ) /* Overflow check */
                /* Ensure that blocks are always aligned. */
                    /* Byte alignment required. Check for overflow. */
                    if( ( xWantedSize + ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) ) ) 
                            > xWantedSize )
                    {
                        xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) );
                        configASSERT( ( xWantedSize & portBYTE_ALIGNMENT_MASK ) == 0 );
                    }
                    else
                    {
                        xWantedSize = 0;
                    }  
            } 
            else 
                xWantedSize = 0;
                 * one of adequate size is found. */
                 * was not found. */
            if( ( xWantedSize > 0 ) && 
                ( ( xWantedSize + xHeapStructSize ) >  xWantedSize ) ) /* Overflow check */
                /* Ensure that blocks are always aligned */
                    /* Byte alignment required. Check for overflow */
                    if( ( xWantedSize + ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) ) ) >
                         xWantedSize )
                    {
                        xWantedSize += ( portBYTE_ALIGNMENT - ( xWantedSize & portBYTE_ALIGNMENT_MASK ) );
                    } 
                    else 
                    {
                        xWantedSize = 0;
                    }
                xWantedSize = 0;
                 * one of adequate size is found. */
                 * was not found. */