Skip to content

Commit bc05723

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 5b6c2f2 commit bc05723

1 file changed

Lines changed: 43 additions & 6 deletions

File tree

stream_buffer.c

Lines changed: 43 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,22 @@ 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+
}
862+
848863
/* If this is a message buffer then it must be possible to write the
849864
* whole message. */
850-
if( xRequiredSpace > xMaxReportedSpace )
865+
else if( xRequiredSpace > xMaxReportedSpace )
851866
{
852867
/* The message would not fit even if the entire buffer was empty,
853868
* so don't wait for space. */
@@ -921,7 +936,10 @@ size_t xStreamBufferSend( StreamBufferHandle_t xStreamBuffer,
921936
mtCOVERAGE_TEST_MARKER();
922937
}
923938

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

926944
if( xReturn > ( size_t ) 0 )
927945
{
@@ -955,8 +973,10 @@ size_t xStreamBufferSendFromISR( StreamBufferHandle_t xStreamBuffer,
955973
BaseType_t * const pxHigherPriorityTaskWoken )
956974
{
957975
StreamBuffer_t * const pxStreamBuffer = xStreamBuffer;
958-
size_t xReturn, xSpace;
976+
size_t xReturn = 0;
977+
size_t xSpace;
959978
size_t xRequiredSpace = xDataLengthBytes;
979+
BaseType_t xSendAllowed = pdTRUE;
960980

961981
traceENTER_xStreamBufferSendFromISR( xStreamBuffer, pvTxData, xDataLengthBytes, pxHigherPriorityTaskWoken );
962982

@@ -973,14 +993,31 @@ size_t xStreamBufferSendFromISR( StreamBufferHandle_t xStreamBuffer,
973993

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

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

9851022
if( xReturn > ( size_t ) 0 )
9861023
{

0 commit comments

Comments
 (0)