Skip to content

Commit c3af983

Browse files
stream_buffer: reject required-space overflow at runtime in message send
Defect: xStreamBufferSend() and xStreamBufferSendFromISR() on a message buffer can perform an out-of-bounds write when the required-space computation for a message overflows size_t and configASSERT() is compiled out. Root cause: for message buffers the code computes xRequiredSpace = xDataLengthBytes + sbBYTES_TO_STORE_MESSAGE_LENGTH and guards it only with configASSERT( xRequiredSpace > xDataLengthBytes ). When the addition wraps size_t, the wrapped (smaller) xRequiredSpace passes the subsequent space checks and the send proceeds to copy xDataLengthBytes bytes, writing past the buffer. With asserts disabled there is no guard at all. Fix: add a runtime check that returns 0 (nothing sent) when xRequiredSpace <= xDataLengthBytes, in both the task and FromISR send paths, so a wrapping message length is rejected regardless of the configASSERT() setting. A host regression test kept outside this repository demonstrates the fault before the change and its absence afterwards (red then green).
1 parent a17db1c commit c3af983

1 file changed

Lines changed: 42 additions & 6 deletions

File tree

stream_buffer.c

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -820,10 +820,12 @@ size_t xStreamBufferSend( StreamBufferHandle_t xStreamBuffer,
820820
TickType_t xTicksToWait )
821821
{
822822
StreamBuffer_t * const pxStreamBuffer = xStreamBuffer;
823-
size_t xReturn, xSpace = 0;
823+
size_t xReturn = 0;
824+
size_t xSpace = 0;
824825
size_t xRequiredSpace = xDataLengthBytes;
825826
TimeOut_t xTimeOut;
826827
size_t xMaxReportedSpace = 0;
828+
BaseType_t xSendAllowed = pdTRUE;
827829

828830
traceENTER_xStreamBufferSend( xStreamBuffer, pvTxData, xDataLengthBytes, xTicksToWait );
829831

@@ -845,9 +847,21 @@ size_t xStreamBufferSend( StreamBufferHandle_t xStreamBuffer,
845847
/* Overflow? */
846848
configASSERT( xRequiredSpace > xDataLengthBytes );
847849

850+
/* Enforce the overflow check at runtime as well, so that a message
851+
* length whose required-space addition wraps size_t is rejected even
852+
* when configASSERT() is compiled out. Without this the wrapped
853+
* (smaller) xRequiredSpace would pass the space checks below and the
854+
* send would proceed to copy xDataLengthBytes bytes, causing an
855+
* out-of-bounds write. Reject the send instead: leave xReturn 0, skip
856+
* blocking and the write, and exit through the single return below. */
857+
if( xRequiredSpace <= xDataLengthBytes )
858+
{
859+
xSendAllowed = pdFALSE;
860+
xTicksToWait = ( TickType_t ) 0;
861+
}
848862
/* If this is a message buffer then it must be possible to write the
849863
* whole message. */
850-
if( xRequiredSpace > xMaxReportedSpace )
864+
else if( xRequiredSpace > xMaxReportedSpace )
851865
{
852866
/* The message would not fit even if the entire buffer was empty,
853867
* so don't wait for space. */
@@ -921,7 +935,10 @@ size_t xStreamBufferSend( StreamBufferHandle_t xStreamBuffer,
921935
mtCOVERAGE_TEST_MARKER();
922936
}
923937

924-
xReturn = prvWriteMessageToBuffer( pxStreamBuffer, pvTxData, xDataLengthBytes, xSpace, xRequiredSpace );
938+
if( xSendAllowed == pdTRUE )
939+
{
940+
xReturn = prvWriteMessageToBuffer( pxStreamBuffer, pvTxData, xDataLengthBytes, xSpace, xRequiredSpace );
941+
}
925942

926943
if( xReturn > ( size_t ) 0 )
927944
{
@@ -955,8 +972,10 @@ size_t xStreamBufferSendFromISR( StreamBufferHandle_t xStreamBuffer,
955972
BaseType_t * const pxHigherPriorityTaskWoken )
956973
{
957974
StreamBuffer_t * const pxStreamBuffer = xStreamBuffer;
958-
size_t xReturn, xSpace;
975+
size_t xReturn = 0;
976+
size_t xSpace;
959977
size_t xRequiredSpace = xDataLengthBytes;
978+
BaseType_t xSendAllowed = pdTRUE;
960979

961980
traceENTER_xStreamBufferSendFromISR( xStreamBuffer, pvTxData, xDataLengthBytes, pxHigherPriorityTaskWoken );
962981

@@ -973,14 +992,31 @@ size_t xStreamBufferSendFromISR( StreamBufferHandle_t xStreamBuffer,
973992

974993
/* Overflow? */
975994
configASSERT( xRequiredSpace > xDataLengthBytes );
995+
996+
/* Enforce the overflow check at runtime as well (see xStreamBufferSend),
997+
* so a wrapping message length is rejected when configASSERT() is
998+
* compiled out rather than proceeding to an out-of-bounds write. Reject
999+
* the send by leaving xReturn 0 and skipping the write below, so the
1000+
* function still exits through its single return. */
1001+
if( xRequiredSpace <= xDataLengthBytes )
1002+
{
1003+
xSendAllowed = pdFALSE;
1004+
}
1005+
else
1006+
{
1007+
mtCOVERAGE_TEST_MARKER();
1008+
}
9761009
}
9771010
else
9781011
{
9791012
mtCOVERAGE_TEST_MARKER();
9801013
}
9811014

982-
xSpace = xStreamBufferSpacesAvailable( pxStreamBuffer );
983-
xReturn = prvWriteMessageToBuffer( pxStreamBuffer, pvTxData, xDataLengthBytes, xSpace, xRequiredSpace );
1015+
if( xSendAllowed == pdTRUE )
1016+
{
1017+
xSpace = xStreamBufferSpacesAvailable( pxStreamBuffer );
1018+
xReturn = prvWriteMessageToBuffer( pxStreamBuffer, pvTxData, xDataLengthBytes, xSpace, xRequiredSpace );
1019+
}
9841020

9851021
if( xReturn > ( size_t ) 0 )
9861022
{

0 commit comments

Comments
 (0)