configENABLE_HEAP_PROTECTOR implemented in heap_5.c

Oliver Lavery · Oliver Lavery · commit 3888ffb88940 · 2023-08-10T17:02:57.000+01:00
diff --git a/portable/MemMang/heap_5.c b/portable/MemMang/heap_5.c
@@ -104,6 +104,9 @@
 /* Check if adding a and b will result in overflow. */
 #define heapADD_WILL_OVERFLOW( a, b )         ( ( a ) > ( heapSIZE_MAX - ( b ) ) )
 
+/* Check if subtracting a from b will result in underflow. */
+#define heapSUBTRACT_WILL_UNDERFLOW( a, b )         ( ( a ) > ( b ) )
+
 /* MSB of the xBlockSize member of an BlockLink_t structure is used to track
  * the allocation status of a block.  When MSB of the xBlockSize member of
  * an BlockLink_t structure is set then the block belongs to the application.
@@ -124,6 +127,38 @@ typedef struct A_BLOCK_LINK
     size_t xBlockSize;                     /**< The size of the free block. */
 } BlockLink_t;
 
+/* Enabling configENABLE_HEAP_PROTECTOR provides a security enhanced version of heap_4.c that
+ * implements pointer protection using a canary value to reduce the risk of code execution
+ * should a heap buffer overflow vulnerability occur.
+ * It also implements additional arithmetic and bounds checks on heap values.
+ */
+#if ( configENABLE_HEAP_PROTECTOR == 1 )
+/* We require xApplicationGetRandomNumber in order to initialize our canary value */
+    extern BaseType_t xApplicationGetRandomNumber( uint32_t * pulNumber );
+/* Canary value for protecting internal heap pointers */
+    static portPOINTER_SIZE_TYPE xHeapCanary;
+/* Highest and lowest heap addresses for bounds checking */
+    static void * pxHeapHighAddress = NULL;
+    static void * pxHeapLowAddress = NULL;
+
+/* Macro to load/store pxNextFreeBlock pointers to memory. By XORing the
+ * pointers with a random canary value, intentional heap overwrites will
+ * result in randomly unpredictable pointer values. If configASSERT() is defined
+ * out of bound values will result in a safe abend due to bounds checks. */
+#define heapPROTECT_POINTER( pxNextFreeBlock )       ( ( struct A_BLOCK_LINK *) ( ( ( portPOINTER_SIZE_TYPE ) ( pxNextFreeBlock ) ) ^ xHeapCanary ) )
+/* Macro to perform a bounds check of heap pointers such that a heap pointer that
+ * falls outside of ucHeap will trigger an assertion. For heap_5.c we check that
+ * pointers are greater than the lowest address in the lowest region, and less than
+ * the highest address in the highest region. */
+#define heapPROTECT_BOUNDS_CHECK( pxBlock )          configASSERT( pxHeapHighAddress && pxHeapLowAddress && \
+    ( ( void * ) ( pxBlock ) >= pxHeapLowAddress ) && ( ( void * ) ( pxBlock ) < pxHeapHighAddress ) )
+#else
+
+#define heapPROTECT_POINTER( pxNextFreeBlock )       ( pxNextFreeBlock )
+#define heapPROTECT_BOUNDS_CHECK( pxBlock )           do {} while(0)
+
+#endif /* configENABLE_HEAP_PROTECTOR */
+
 /*-----------------------------------------------------------*/
 
 /*
@@ -217,12 +252,14 @@ void * pvPortMalloc( size_t xWantedSize )
                 /* Traverse the list from the start (lowest address) block until
                  * one of adequate size is found. */
                 pxPreviousBlock = &xStart;
-                pxBlock = xStart.pxNextFreeBlock;
+                pxBlock = heapPROTECT_POINTER( xStart.pxNextFreeBlock );
+                heapPROTECT_BOUNDS_CHECK( pxBlock );
 
-                while( ( pxBlock->xBlockSize < xWantedSize ) && ( pxBlock->pxNextFreeBlock != NULL ) )
+                while( ( pxBlock->xBlockSize < xWantedSize ) && ( pxBlock->pxNextFreeBlock != heapPROTECT_POINTER( NULL ) ) )
                 {
                     pxPreviousBlock = pxBlock;
-                    pxBlock = pxBlock->pxNextFreeBlock;
+                    pxBlock = heapPROTECT_POINTER( pxBlock->pxNextFreeBlock );
+                    heapPROTECT_BOUNDS_CHECK( pxBlock );
                 }
 
                 /* If the end marker was reached then a block of adequate size
@@ -231,21 +268,25 @@ void * pvPortMalloc( size_t xWantedSize )
                 {
                     /* Return the memory space pointed to - jumping over the
                      * BlockLink_t structure at its start. */
-                    pvReturn = ( void * ) ( ( ( uint8_t * ) pxPreviousBlock->pxNextFreeBlock ) + xHeapStructSize );
+                    pvReturn = ( void * )  ( ( uint8_t * ) heapPROTECT_POINTER( pxPreviousBlock->pxNextFreeBlock ) ) + xHeapStructSize;
+                    heapPROTECT_BOUNDS_CHECK( pvReturn );
 
                     /* This block is being returned for use so must be taken out
                      * of the list of free blocks. */
                     pxPreviousBlock->pxNextFreeBlock = pxBlock->pxNextFreeBlock;
 
                     /* If the block is larger than required it can be split into
-                     * two. */
+                     * two. Also check for underflow as this can occur if xBlockSize is
+                     * overwritten in a heap block */
+                    configASSERT( !heapSUBTRACT_WILL_UNDERFLOW( xWantedSize, pxBlock->xBlockSize ) );
                     if( ( pxBlock->xBlockSize - xWantedSize ) > heapMINIMUM_BLOCK_SIZE )
                     {
                         /* This block is to be split into two.  Create a new
                          * block following the number of bytes requested. The void
                          * cast is used to prevent byte alignment warnings from the
                          * compiler. */
                         pxNewBlockLink = ( void * ) ( ( ( uint8_t * ) pxBlock ) + xWantedSize );
+                        configASSERT( ( ( ( size_t ) pxNewBlockLink ) & portBYTE_ALIGNMENT_MASK ) == 0 );
 
                         /* Calculate the sizes of two blocks split from the
                          * single block. */
@@ -254,7 +295,7 @@ void * pvPortMalloc( size_t xWantedSize )
 
                         /* Insert the new block into the list of free blocks. */
                         pxNewBlockLink->pxNextFreeBlock = pxPreviousBlock->pxNextFreeBlock;
-                        pxPreviousBlock->pxNextFreeBlock = pxNewBlockLink;
+                        pxPreviousBlock->pxNextFreeBlock = heapPROTECT_POINTER( pxNewBlockLink );
                     }
                     else
                     {
@@ -310,6 +351,7 @@ void * pvPortMalloc( size_t xWantedSize )
     }
     #endif /* if ( configUSE_MALLOC_FAILED_HOOK == 1 ) */
 
+    configASSERT( ( ( ( size_t ) pvReturn ) & ( size_t ) portBYTE_ALIGNMENT_MASK ) == 0 );
     return pvReturn;
 }
 /*-----------------------------------------------------------*/
@@ -327,7 +369,8 @@ void vPortFree( void * pv )
 
         /* This casting is to keep the compiler from issuing warnings. */
         pxLink = ( void * ) puc;
-
+        /* This will assert if pv is out of heap range */
+        heapPROTECT_BOUNDS_CHECK( pxLink );
         configASSERT( heapBLOCK_IS_ALLOCATED( pxLink ) != 0 );
         configASSERT( pxLink->pxNextFreeBlock == NULL );
 
@@ -340,6 +383,9 @@ void vPortFree( void * pv )
                 heapFREE_BLOCK( pxLink );
                 #if ( configHEAP_CLEAR_MEMORY_ON_FREE == 1 )
                 {
+                    /* Check for underflow as this can occur if xBlockSize is
+                     * overwritten in a heap block */
+                    configASSERT( !heapSUBTRACT_WILL_UNDERFLOW( xHeapStructSize, pxLink->xBlockSize ) );
                     ( void ) memset( puc + xHeapStructSize, 0, pxLink->xBlockSize - xHeapStructSize );
                 }
                 #endif
@@ -403,12 +449,18 @@ static void prvInsertBlockIntoFreeList( BlockLink_t * pxBlockToInsert )
     BlockLink_t * pxIterator;
     uint8_t * puc;
 
+    /* Ensure the block to insert is within the heap region */
+    heapPROTECT_BOUNDS_CHECK( pxBlockToInsert );
+
     /* Iterate through the list until a block is found that has a higher address
      * than the block being inserted. */
-    for( pxIterator = &xStart; pxIterator->pxNextFreeBlock < pxBlockToInsert; pxIterator = pxIterator->pxNextFreeBlock )
+    for( pxIterator = &xStart; heapPROTECT_POINTER( pxIterator->pxNextFreeBlock ) < pxBlockToInsert; pxIterator = heapPROTECT_POINTER( pxIterator->pxNextFreeBlock ) )
     {
         /* Nothing to do here, just iterate to the right position. */
     }
+    if ( pxIterator != &xStart ) {
+        heapPROTECT_BOUNDS_CHECK( pxIterator );
+    }
 
     /* Do the block being inserted, and the block it is being inserted after
      * make a contiguous block of memory? */
@@ -428,17 +480,17 @@ static void prvInsertBlockIntoFreeList( BlockLink_t * pxBlockToInsert )
      * make a contiguous block of memory? */
     puc = ( uint8_t * ) pxBlockToInsert;
 
-    if( ( puc + pxBlockToInsert->xBlockSize ) == ( uint8_t * ) pxIterator->pxNextFreeBlock )
+    if( ( puc + pxBlockToInsert->xBlockSize ) == ( uint8_t * ) heapPROTECT_POINTER( pxIterator->pxNextFreeBlock ) )
     {
-        if( pxIterator->pxNextFreeBlock != pxEnd )
+        if( heapPROTECT_POINTER( pxIterator->pxNextFreeBlock ) != pxEnd )
         {
             /* Form one big block from the two blocks. */
-            pxBlockToInsert->xBlockSize += pxIterator->pxNextFreeBlock->xBlockSize;
-            pxBlockToInsert->pxNextFreeBlock = pxIterator->pxNextFreeBlock->pxNextFreeBlock;
+            pxBlockToInsert->xBlockSize += heapPROTECT_POINTER( pxIterator->pxNextFreeBlock )->xBlockSize;
+            pxBlockToInsert->pxNextFreeBlock = heapPROTECT_POINTER( pxIterator->pxNextFreeBlock )->pxNextFreeBlock;
         }
         else
         {
-            pxBlockToInsert->pxNextFreeBlock = pxEnd;
+            pxBlockToInsert->pxNextFreeBlock = heapPROTECT_POINTER( pxEnd );
         }
     }
     else
@@ -452,7 +504,7 @@ static void prvInsertBlockIntoFreeList( BlockLink_t * pxBlockToInsert )
      * to itself. */
     if( pxIterator != pxBlockToInsert )
     {
-        pxIterator->pxNextFreeBlock = pxBlockToInsert;
+        pxIterator->pxNextFreeBlock = heapPROTECT_POINTER( pxBlockToInsert );
     }
     else
     {
@@ -470,10 +522,30 @@ void vPortDefineHeapRegions( const HeapRegion_t * const pxHeapRegions )
     BaseType_t xDefinedRegions = 0;
     portPOINTER_SIZE_TYPE xAddress;
     const HeapRegion_t * pxHeapRegion;
+#if ( configENABLE_HEAP_PROTECTOR == 1)
+    uint32_t xRandom;
+#endif
 
     /* Can only call once! */
     configASSERT( pxEnd == NULL );
 
+#if ( configENABLE_HEAP_PROTECTOR == 1)
+    switch( sizeof( portPOINTER_SIZE_TYPE ) ) {
+        case 2:
+        case 4:
+            xApplicationGetRandomNumber( &xRandom );
+            xHeapCanary = ( portPOINTER_SIZE_TYPE ) xRandom;
+            break;
+        default:
+        case 8:
+            xApplicationGetRandomNumber( &xRandom );
+            xHeapCanary =  ( ( portPOINTER_SIZE_TYPE ) xRandom ) << sizeof( uint32_t ) * heapBITS_PER_BYTE;
+            xApplicationGetRandomNumber( &xRandom );
+            xHeapCanary |= ( portPOINTER_SIZE_TYPE ) xRandom;
+            break;
+    }
+#endif
+
     pxHeapRegion = &( pxHeapRegions[ xDefinedRegions ] );
 
     while( pxHeapRegion->xSizeInBytes > 0 )
@@ -499,14 +571,17 @@ void vPortDefineHeapRegions( const HeapRegion_t * const pxHeapRegions )
         {
             /* xStart is used to hold a pointer to the first item in the list of
              *  free blocks.  The void cast is used to prevent compiler warnings. */
-            xStart.pxNextFreeBlock = ( BlockLink_t * ) xAlignedHeap;
+            xStart.pxNextFreeBlock = ( BlockLink_t * ) heapPROTECT_POINTER( xAlignedHeap );
             xStart.xBlockSize = ( size_t ) 0;
+#if ( configENABLE_HEAP_PROTECTOR == 1 )
+            pxHeapLowAddress = ( void * ) xAlignedHeap;
+#endif
         }
         else
         {
             /* Should only get here if one region has already been added to the
              * heap. */
-            configASSERT( pxEnd != NULL );
+            configASSERT( pxEnd != heapPROTECT_POINTER( NULL ) );
 
             /* Check blocks are passed in with increasing start addresses. */
             configASSERT( ( size_t ) xAddress > ( size_t ) pxEnd );
@@ -523,24 +598,27 @@ void vPortDefineHeapRegions( const HeapRegion_t * const pxHeapRegions )
         xAddress &= ~( ( portPOINTER_SIZE_TYPE ) portBYTE_ALIGNMENT_MASK );
         pxEnd = ( BlockLink_t * ) xAddress;
         pxEnd->xBlockSize = 0;
-        pxEnd->pxNextFreeBlock = NULL;
+        pxEnd->pxNextFreeBlock = heapPROTECT_POINTER( NULL );
 
         /* To start with there is a single free block in this region that is
          * sized to take up the entire heap region minus the space taken by the
          * free block structure. */
         pxFirstFreeBlockInRegion = ( BlockLink_t * ) xAlignedHeap;
         pxFirstFreeBlockInRegion->xBlockSize = ( size_t ) ( xAddress - ( portPOINTER_SIZE_TYPE ) pxFirstFreeBlockInRegion );
-        pxFirstFreeBlockInRegion->pxNextFreeBlock = pxEnd;
+        pxFirstFreeBlockInRegion->pxNextFreeBlock = heapPROTECT_POINTER( pxEnd );
 
         /* If this is not the first region that makes up the entire heap space
          * then link the previous region to this region. */
         if( pxPreviousFreeBlock != NULL )
         {
-            pxPreviousFreeBlock->pxNextFreeBlock = pxFirstFreeBlockInRegion;
+            pxPreviousFreeBlock->pxNextFreeBlock = heapPROTECT_POINTER( pxFirstFreeBlockInRegion );
         }
 
         xTotalHeapSize += pxFirstFreeBlockInRegion->xBlockSize;
 
+#if ( configENABLE_HEAP_PROTECTOR == 1 )
+        pxHeapHighAddress = ( ( uint8_t *  )pxFirstFreeBlockInRegion ) + pxFirstFreeBlockInRegion->xBlockSize;
+#endif
         /* Move onto the next HeapRegion_t structure. */
         xDefinedRegions++;
         pxHeapRegion = &( pxHeapRegions[ xDefinedRegions ] );
@@ -561,7 +639,7 @@ void vPortGetHeapStats( HeapStats_t * pxHeapStats )
 
     vTaskSuspendAll();
     {
-        pxBlock = xStart.pxNextFreeBlock;
+        pxBlock = heapPROTECT_POINTER( xStart.pxNextFreeBlock );
 
         /* pxBlock will be NULL if the heap has not been initialised.  The heap
          * is initialised automatically when the first allocation is made. */
@@ -591,7 +669,7 @@ void vPortGetHeapStats( HeapStats_t * pxHeapStats )
 
                 /* Move to the next block in the chain until the last block is
                  * reached. */
-                pxBlock = pxBlock->pxNextFreeBlock;
+                pxBlock = heapPROTECT_POINTER( pxBlock->pxNextFreeBlock );
             }
         }
     }
