Possible buffer overflow in FF_Write() after FF_BlockWrite() error

Libraries
1

Hello,

I believe I found a bug in FreeRTOS+FAT, in ff_file.c.

As I understand the logic, FF_Write() writes full sectors using FF_BlockWrite(), and then writes the remaining tail bytes using FF_WritePartial().

However, if FF_BlockWrite() returns an error inside this loop:

while( ulBytesLeft >= ( uint32_t ) pxIOManager->usSectorSize )

the code breaks out of the loop without reducing ulBytesLeft. As a result, execution can continue to the “remaining bytes” path with ulBytesLeft still greater than or equal to the sector size.

That leads to this call:

FF_WritePartial( pxFile, ulItemLBA, 0, ulBytesLeft, pucBuffer, &xError );

In this case, ulBytesLeft may be larger than one sector. Then, inside FF_WritePartial(), the following memcpy() can overflow the sector buffer and cause memory corruption:

memcpy( pxFile->pucBuffer + ulRelBlockPos, pucBuffer, ulCount );

So the issue seems to be that FF_WritePartial() is reached with ulCount >= usSectorSize when FF_BlockWrite() fails.

My expectation is that after a FF_BlockWrite() error, FF_Write() should either:

  1. return immediately with the error, or
  2. ensure that FF_WritePartial() is only called when ulBytesLeft is strictly less than one sector.

Relevant code path:

while( ulBytesLeft >= ( uint32_t ) pxIOManager->usSectorSize )
{
ulSectors = ulBytesLeft / pxIOManager->usSectorSize;
{
FF_Partition_t *pPart = &( pxIOManager->xPartition );
uint32_t ulOffset = ( pxFile->ulFilePointer / pxIOManager->usSectorSize ) % pPart->ulSectorsPerCluster;
uint32_t ulRemain = pPart->ulSectorsPerCluster - ulOffset;
if( ulSectors > ulRemain )
{
ulSectors = ulRemain;
}
}

ulItemLBA = FF_SetCluster( pxFile, &xError );
if( FF_isERR( xError ) )
{
    break;
}

xError = FF_BlockWrite( pxIOManager, ulItemLBA, ulSectors, pucBuffer, pdFALSE );
if( FF_isERR( xError ) )
{
    break;
}

nBytesToWrite = ulSectors * pxIOManager->usSectorSize;
ulBytesLeft -= nBytesToWrite;
pucBuffer += nBytesToWrite;
nBytesWritten += nBytesToWrite;
pxFile->ulFilePointer += nBytesToWrite;
if( pxFile->ulFilePointer > pxFile->ulFileSize )
{
    pxFile->ulFileSize = pxFile->ulFilePointer;
}

}

/*---------- Write (memcpy) Remaining Bytes */
if( ulBytesLeft == 0 )
{
break;
}

ulItemLBA = FF_SetCluster( pxFile, &xError );
if( FF_isERR( xError ) )
{
break;
}
FF_WritePartial( pxFile, ulItemLBA, 0, ulBytesLeft, pucBuffer, &xError );
nBytesWritten += ulBytesLeft;

Could you please confirm whether this is a valid bug?

Thank you.

2

but there is an

    if( FF_isERR( xError ) )
    {
        break;
    }

after the do while loop, so the partial_write should never be reached

Lab-Project-FreeRTOS-FAT/ff_file.c at main · FreeRTOS/Lab-Project-FreeRTOS-FAT · GitHub

3

It appears that this issue was already fixed in the commit from Jul 9, 2024. We will update our side to use the latest version.

Thank you very much for your support.

Also, could you please let me know whether there is a recommended way to stay informed about FreeRTOS+FAT updates or bug fixes? For example, should we watch the GitHub repository or follow announcements somewhere?

4

Yes, watching the repo is the right thing to do to stay informed about the updates.