Skip to content

Commit

Permalink
test_vTCPWindowDestroy_list_length_not_zero(): Fix buffer overflow du…
Browse files Browse the repository at this point in the history
…e to struct interposing

The test was crashing due to what AddressSanitizer calls a buffer overflow,
or really, interposing a TCPSegment_t on top of a TCPWindow_t::xRxSegments
member and accessing an interposed struct member that fell outside the
underlying TCPWindow_t struct.

The naive fix - not doing that - works:

     void test_vTCPWindowDestroy_list_length_not_zero( void )
     {
         TCPWindow_t xWindow = { 0 };
    -    List_t * pxSegments = &( xWindow.xRxSegments );
    +    TCPSegment_t xSegment = { 0 };

         listLIST_IS_INITIALISED_ExpectAnyArgsAndReturn( pdFALSE );
         listLIST_IS_INITIALISED_ExpectAnyArgsAndReturn( pdTRUE );
         listCURRENT_LIST_LENGTH_ExpectAnyArgsAndReturn( 1 );
    -    listGET_OWNER_OF_HEAD_ENTRY_ExpectAnyArgsAndReturn( pxSegments );
    +    listGET_OWNER_OF_HEAD_ENTRY_ExpectAnyArgsAndReturn( &xSegment );
         /* ->vTCPWindowFree */
    -    uxListRemove_ExpectAnyArgsAndReturn( pdTRUE );
    -    uxListRemove_ExpectAnyArgsAndReturn( pdTRUE );
         listCURRENT_LIST_LENGTH_ExpectAnyArgsAndReturn( 0 );

         vTCPWindowDestroy( &xWindow );
     }

However, this became a different test, as evidenced by the less than 100%
line coverage, that two function call expectations had to go, and that it
functionally became an exact copy of the next test.
To reach the holes in the test coverage opened by the naive fix,
the two list items' container pointers also needed and sufficed to be set.
  • Loading branch information
anordal committed Jun 4, 2024
1 parent 8b9b6f7 commit c3048cc
Showing 1 changed file with 5 additions and 2 deletions.
7 changes: 5 additions & 2 deletions test/unit-test/FreeRTOS_TCP_WIN/FreeRTOS_TCP_WIN_utest.c
Original file line number Diff line number Diff line change
Expand Up @@ -193,12 +193,15 @@ void test_vTCPWindowDestroy_list_length_zero( void )
void test_vTCPWindowDestroy_list_length_not_zero( void )
{
TCPWindow_t xWindow = { 0 };
List_t * pxSegments = &( xWindow.xRxSegments );
TCPSegment_t xSegment = { 0 };

xSegment.xQueueItem.pvContainer = &xWindow.xPriorityQueue;
xSegment.xSegmentItem.pvContainer = &xWindow.xPriorityQueue;

listLIST_IS_INITIALISED_ExpectAnyArgsAndReturn( pdFALSE );
listLIST_IS_INITIALISED_ExpectAnyArgsAndReturn( pdTRUE );
listCURRENT_LIST_LENGTH_ExpectAnyArgsAndReturn( 1 );
listGET_OWNER_OF_HEAD_ENTRY_ExpectAnyArgsAndReturn( pxSegments );
listGET_OWNER_OF_HEAD_ENTRY_ExpectAnyArgsAndReturn( &xSegment );
/* ->vTCPWindowFree */
uxListRemove_ExpectAnyArgsAndReturn( pdTRUE );
uxListRemove_ExpectAnyArgsAndReturn( pdTRUE );
Expand Down

0 comments on commit c3048cc

Please sign in to comment.