Skip to content

Commit

Permalink
[1] FreeRTOS_DHCP_utest: Fix systematic memory errors in test input
Browse files Browse the repository at this point in the history
The trivial problem: Screenfuls of misaligned store warnings:

    Store to misaligned address 0x7ffe* for type 'uint32_t',
    which requires 4 byte alignment

I assume there is no reason not to convert these to memcpy.
I have done that when that was the only problem I saw.

The more involved problem: The length fields, buffer sizes and
stop byte (0xFF) offsets have fallen out of maintenance.
This would lead the DHCP option parser under test to read out
of bounds when it overshot the stop byte. Symptom:

    AddressSanitizer: dynamic-stack-buffer-overflow on address 0x7ffc9dfa5c07
    READ of size 1 at 0x7ffc9dfa5c07 thread T0
    #0 0x459a49 in prvProcessDHCPReplies test/unit-test/build/Annexed_TCP_Sources/FreeRTOS_DHCP.c:1310
    FreeRTOS#1 0x4526d2 in vHandleWaitingAcknowledge test/unit-test/build/Annexed_TCP_Sources/FreeRTOS_DHCP.c:495
    FreeRTOS#2 0x4544ef in vDHCPProcessEndPoint test/unit-test/build/Annexed_TCP_Sources/FreeRTOS_DHCP.c:739
    FreeRTOS#3 0x43dbd9 in test_vDHCPProcess_eWaitingAcknowledge_DNSIncorrectLength2
        (test/unit-test/build/bin/tests/FreeRTOS_DHCP_utest+0x43dbd9)

    Address 0x7ffc9dfa5c07 is located in stack of thread T0
    SUMMARY: AddressSanitizer: dynamic-stack-buffer-overflow
        test/unit-test/build/Annexed_TCP_Sources/FreeRTOS_DHCP.c:1310 in prvProcessDHCPReplies
    Shadow bytes around the buggy address:
    0x7ffc9dfa5980: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    0x7ffc9dfa5a00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    0x7ffc9dfa5a80: 00 00 00 00 00 00 00 00 ca ca ca ca 00 00 00 00
    0x7ffc9dfa5b00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    0x7ffc9dfa5b80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    =>0x7ffc9dfa5c00:[05]cb cb cb cb cb cb cb 00 00 00 00 00 00 00 00
    0x7ffc9dfa5c80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    0x7ffc9dfa5d00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    0x7ffc9dfa5d80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    0x7ffc9dfa5e00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    0x7ffc9dfa5e80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    Shadow byte legend (one shadow byte represents 8 application bytes):
    Addressable:           00
    Partially addressable: 01 02 03 04 05 06 07
    Left alloca redzone:   ca
    Right alloca redzone:  cb

I think this calls for some helper functions, which I've added.
These eliminate the possibility for such inconsistencies,
drastically cuts down on low-entropy code,
and by wrapping memcpy, also avoids the first problem.
I have done that instead wherever I also saw this kind of inconsistencies.
  • Loading branch information
anordal committed May 26, 2024
1 parent f0816e0 commit 943ebeb
Showing 1 changed file with 33 additions and 33 deletions.
66 changes: 33 additions & 33 deletions test/unit-test/FreeRTOS_DHCP/FreeRTOS_DHCP_utest.c
Original file line number Diff line number Diff line change
Expand Up @@ -2654,7 +2654,7 @@ void test_vDHCPProcess_eWaitingOfferCorrectDHCPMessageTwoOptionsSendFails( void
/* Add length. */
DHCPOption[ 1 ] = 4;
/* Add the offer byte. */
*( ( uint32_t * ) &DHCPOption[ 2 ] ) = DHCPServerAddress;
memcpy( &DHCPOption[ 2 ], &DHCPServerAddress, sizeof( DHCPServerAddress ) );


/* Put the information in global variables to be returned by
Expand Down Expand Up @@ -2751,7 +2751,7 @@ void test_vDHCPProcess_eWaitingOfferCorrectDHCPMessageTwoOptionsSendSucceeds( vo
/* Add length. */
DHCPOption[ 1 ] = 4;
/* Add the offer byte. */
*( ( uint32_t * ) &DHCPOption[ 2 ] ) = DHCPServerAddress;
memcpy( &DHCPOption[ 2 ], &DHCPServerAddress, sizeof( DHCPServerAddress ) );


/* Put the information in global variables to be returned by
Expand Down Expand Up @@ -2853,7 +2853,7 @@ void test_vDHCPProcess_eWaitingOfferCorrectDHCPMessageTwoOptionsDHCPHookReturnDe
/* Add length. */
DHCPOption[ 1 ] = 4;
/* Add the offer byte. */
*( ( uint32_t * ) &DHCPOption[ 2 ] ) = DHCPServerAddress;
memcpy( &DHCPOption[ 2 ], &DHCPServerAddress, sizeof( DHCPServerAddress ) );


/* Put the information in global variables to be returned by
Expand Down Expand Up @@ -2952,7 +2952,7 @@ void test_vDHCPProcess_eWaitingOfferCorrectDHCPMessageTwoOptionsDHCPHookReturnEr
/* Add length. */
DHCPOption[ 1 ] = 4;
/* Add the offer byte. */
*( ( uint32_t * ) &DHCPOption[ 2 ] ) = DHCPServerAddress;
memcpy( &DHCPOption[ 2 ], &DHCPServerAddress, sizeof( DHCPServerAddress ) );


/* Put the information in global variables to be returned by
Expand Down Expand Up @@ -3052,7 +3052,7 @@ void test_vDHCPProcess_eWaitingAcknowledgeTwoOptionsIncorrectServerNoTimeout( vo
/* Add length. */
DHCPOption[ 1 ] = 4;
/* Add the offer byte. */
*( ( uint32_t * ) &DHCPOption[ 2 ] ) = DHCPServerAddress;
memcpy( &DHCPOption[ 2 ], &DHCPServerAddress, sizeof( DHCPServerAddress ) );

/* Put the information in global variables to be returned by
* the FreeRTOS_recvrom. */
Expand Down Expand Up @@ -3137,7 +3137,7 @@ void test_vDHCPProcess_eWaitingAcknowledgeTwoOptionsIncorrectServerTimeoutGNBfai
/* Add length. */
DHCPOption[ 1 ] = 4;
/* Add the offer byte. */
*( ( uint32_t * ) &DHCPOption[ 2 ] ) = DHCPServerAddress;
memcpy( &DHCPOption[ 2 ], &DHCPServerAddress, sizeof( DHCPServerAddress ) );

/* Put the information in global variables to be returned by
* the FreeRTOS_recvrom. */
Expand Down Expand Up @@ -3227,7 +3227,7 @@ void test_vDHCPProcess_eWaitingAcknowledgeTwoOptionsIncorrectServerTimeoutGNBSuc
/* Add length. */
DHCPOption[ 1 ] = 4;
/* Add the offer byte. */
*( ( uint32_t * ) &DHCPOption[ 2 ] ) = DHCPServerAddress;
memcpy( &DHCPOption[ 2 ], &DHCPServerAddress, sizeof( DHCPServerAddress ) );


/* Put the information in global variables to be returned by
Expand Down Expand Up @@ -3322,7 +3322,7 @@ void test_vDHCPProcess_eWaitingAcknowledgeTwoOptionsIncorrectServerTimeoutPeriod
/* Add length. */
DHCPOption[ 1 ] = 4;
/* Add the offer byte. */
*( ( uint32_t * ) &DHCPOption[ 2 ] ) = DHCPServerAddress;
memcpy( &DHCPOption[ 2 ], &DHCPServerAddress, sizeof( DHCPServerAddress ) );


/* Put the information in global variables to be returned by
Expand Down Expand Up @@ -3407,7 +3407,7 @@ void test_vDHCPProcess_eWaitingAcknowledgeTwoOptionsIncorrectServerTimeoutPeriod
/* Add length. */
DHCPOption[ 1 ] = 4;
/* Add the offer byte. */
*( ( uint32_t * ) &DHCPOption[ 2 ] ) = DHCPServerAddress;
memcpy( &DHCPOption[ 2 ], &DHCPServerAddress, sizeof( DHCPServerAddress ) );


/* Put the information in global variables to be returned by
Expand Down Expand Up @@ -3493,7 +3493,7 @@ void test_vDHCPProcess_eWaitingAcknowledgeTwoOptionsCorrectServerLeaseTimeZero(
/* Add length. */
DHCPOption[ 1 ] = 4;
/* Add the offer byte. */
*( ( uint32_t * ) &DHCPOption[ 2 ] ) = DHCPServerAddress;
memcpy( &DHCPOption[ 2 ], &DHCPServerAddress, sizeof( DHCPServerAddress ) );


/* Put the information in global variables to be returned by
Expand Down Expand Up @@ -3594,7 +3594,7 @@ void test_vDHCPProcess_eWaitingAcknowledgeTwoOptionsCorrectServerLeaseTimeLessTh
/* Add length. */
DHCPOption[ 1 ] = 4;
/* Add the offer byte. */
*( ( uint32_t * ) &DHCPOption[ 2 ] ) = DHCPServerAddress;
memcpy( &DHCPOption[ 2 ], &DHCPServerAddress, sizeof( DHCPServerAddress ) );


/* Put the information in global variables to be returned by
Expand Down Expand Up @@ -3693,7 +3693,7 @@ void test_vDHCPProcess_eWaitingAcknowledge_TwoOptions_CorrectServer_AptLeaseTime
/* Add length. */
DHCPOption[ 1 ] = 4;
/* Add the offer byte. */
*( ( uint32_t * ) &DHCPOption[ 2 ] ) = DHCPServerAddress;
memcpy( &DHCPOption[ 2 ], &DHCPServerAddress, sizeof( DHCPServerAddress ) );


/* Put the information in global variables to be returned by
Expand Down Expand Up @@ -3791,7 +3791,7 @@ void test_vDHCPProcess_eWaitingAcknowledge_TwoOptions_NACK( void )
/* Add length. */
DHCPOption[ 1 ] = 4;
/* Add the offer byte. */
*( ( uint32_t * ) &DHCPOption[ 2 ] ) = DHCPServerAddress;
memcpy( &DHCPOption[ 2 ], &DHCPServerAddress, sizeof( DHCPServerAddress ) );


/* Put the information in global variables to be returned by
Expand Down Expand Up @@ -3878,7 +3878,7 @@ void test_vDHCPProcess_eWaitingAcknowledge_TwoOptions_OFFER( void )
/* Add length. */
DHCPOption[ 1 ] = 4;
/* Add the offer byte. */
*( ( uint32_t * ) &DHCPOption[ 2 ] ) = DHCPServerAddress;
memcpy( &DHCPOption[ 2 ], &DHCPServerAddress, sizeof( DHCPServerAddress ) );


/* Put the information in global variables to be returned by
Expand Down Expand Up @@ -3980,39 +3980,39 @@ void test_vDHCPProcess_eWaitingAcknowledge_AllOptionsCorrectLength( void )
/* Add length. */
DHCPOption[ 1 ] = 4;
/* Add the offer byte. */
*( ( uint32_t * ) &DHCPOption[ 2 ] ) = DHCPServerAddress;
memcpy( &DHCPOption[ 2 ], &DHCPServerAddress, sizeof( DHCPServerAddress ) );

DHCPOption += 6;
/* Add Message type code. */
DHCPOption[ 0 ] = dhcpIPv4_SUBNET_MASK_OPTION_CODE;
/* Add length. */
DHCPOption[ 1 ] = 4;
/* Add the offer byte. */
*( ( uint32_t * ) &DHCPOption[ 2 ] ) = ulSubnetMask;
memcpy( &DHCPOption[ 2 ], &ulSubnetMask, sizeof( ulSubnetMask ) );

DHCPOption += 6;
/* Add Message type code. */
DHCPOption[ 0 ] = dhcpIPv4_GATEWAY_OPTION_CODE;
/* Add length. */
DHCPOption[ 1 ] = 4;
/* Add the offer byte. */
*( ( uint32_t * ) &DHCPOption[ 2 ] ) = ulGateway;
memcpy( &DHCPOption[ 2 ], &ulGateway, sizeof( ulGateway ) );

DHCPOption += 6;
/* Add Message type code. */
DHCPOption[ 0 ] = dhcpIPv4_LEASE_TIME_OPTION_CODE;
/* Add length. */
DHCPOption[ 1 ] = 4;
/* Add the offer byte. */
*( ( uint32_t * ) &DHCPOption[ 2 ] ) = ulLeaseTime;
memcpy( &DHCPOption[ 2 ], &ulLeaseTime, sizeof( ulLeaseTime ) );

DHCPOption += 6;
/* Add Message type code. */
DHCPOption[ 0 ] = dhcpIPv4_DNS_SERVER_OPTIONS_CODE;
/* Add length. */
DHCPOption[ 1 ] = 4;
/* Add the offer byte. */
*( ( uint32_t * ) &DHCPOption[ 2 ] ) = ulDNSServer;
memcpy( &DHCPOption[ 2 ], &ulDNSServer, sizeof( ulDNSServer ) );


/* Put the information in global variables to be returned by
Expand Down Expand Up @@ -4126,39 +4126,39 @@ void test_vDHCPProcess_eWaitingAcknowledge_AllOptionsCorrectLength2( void )
/* Add length. */
DHCPOption[ 1 ] = 4;
/* Add the offer byte. */
*( ( uint32_t * ) &DHCPOption[ 2 ] ) = DHCPServerAddress;
memcpy( &DHCPOption[ 2 ], &DHCPServerAddress, sizeof( DHCPServerAddress ) );

DHCPOption += 6;
/* Add Message type code. */
DHCPOption[ 0 ] = dhcpIPv4_SUBNET_MASK_OPTION_CODE;
/* Add length. */
DHCPOption[ 1 ] = 4;
/* Add the offer byte. */
*( ( uint32_t * ) &DHCPOption[ 2 ] ) = ulSubnetMask;
memcpy( &DHCPOption[ 2 ], &ulSubnetMask, sizeof( ulSubnetMask ) );

DHCPOption += 6;
/* Add Message type code. */
DHCPOption[ 0 ] = dhcpIPv4_GATEWAY_OPTION_CODE;
/* Add length. */
DHCPOption[ 1 ] = 4;
/* Add the offer byte. */
*( ( uint32_t * ) &DHCPOption[ 2 ] ) = ulGateway;
memcpy( &DHCPOption[ 2 ], &ulGateway, sizeof( ulGateway ) );

DHCPOption += 6;
/* Add Message type code. */
DHCPOption[ 0 ] = dhcpIPv4_LEASE_TIME_OPTION_CODE;
/* Add length. */
DHCPOption[ 1 ] = 4;
/* Add the offer byte. */
*( ( uint32_t * ) &DHCPOption[ 2 ] ) = ulLeaseTime;
memcpy( &DHCPOption[ 2 ], &ulLeaseTime, sizeof( ulLeaseTime ) );

DHCPOption += 6;
/* Add Message type code. */
DHCPOption[ 0 ] = dhcpIPv4_DNS_SERVER_OPTIONS_CODE;
/* Add length. */
DHCPOption[ 1 ] = 4;
/* Add the offer byte. */
*( ( uint32_t * ) &DHCPOption[ 2 ] ) = ulDNSServer;
memcpy( &DHCPOption[ 2 ], &ulDNSServer, sizeof( ulDNSServer ) );


/* Put the information in global variables to be returned by
Expand Down Expand Up @@ -4273,39 +4273,39 @@ void test_vDHCPProcess_eWaitingAcknowledge_DNSIncorrectLength( void )
/* Add length. */
DHCPOption[ 1 ] = 4;
/* Add the offer byte. */
*( ( uint32_t * ) &DHCPOption[ 2 ] ) = DHCPServerAddress;
memcpy( &DHCPOption[ 2 ], &DHCPServerAddress, sizeof( DHCPServerAddress ) );

DHCPOption += 6;
/* Add Message type code. */
DHCPOption[ 0 ] = dhcpIPv4_SUBNET_MASK_OPTION_CODE;
/* Add length. */
DHCPOption[ 1 ] = 4;
/* Add the offer byte. */
*( ( uint32_t * ) &DHCPOption[ 2 ] ) = ulSubnetMask;
memcpy( &DHCPOption[ 2 ], &ulSubnetMask, sizeof( ulSubnetMask ) );

DHCPOption += 6;
/* Add Message type code. */
DHCPOption[ 0 ] = dhcpIPv4_GATEWAY_OPTION_CODE;
/* Add length. */
DHCPOption[ 1 ] = 4;
/* Add the offer byte. */
*( ( uint32_t * ) &DHCPOption[ 2 ] ) = ulGateway;
memcpy( &DHCPOption[ 2 ], &ulGateway, sizeof( ulGateway ) );

DHCPOption += 6;
/* Add Message type code. */
DHCPOption[ 0 ] = dhcpIPv4_LEASE_TIME_OPTION_CODE;
/* Add length. */
DHCPOption[ 1 ] = 4;
/* Add the offer byte. */
*( ( uint32_t * ) &DHCPOption[ 2 ] ) = ulLeaseTime;
memcpy( &DHCPOption[ 2 ], &ulLeaseTime, sizeof( ulLeaseTime ) );

DHCPOption += 6;
/* Add Message type code. */
DHCPOption[ 0 ] = dhcpIPv4_DNS_SERVER_OPTIONS_CODE;
/* Add length. */
DHCPOption[ 1 ] = 3;
/* Add the offer byte. */
*( ( uint32_t * ) &DHCPOption[ 2 ] ) = ulDNSServer;
memcpy( &DHCPOption[ 2 ], &ulDNSServer, sizeof( ulDNSServer ) );


/* Put the information in global variables to be returned by
Expand Down Expand Up @@ -4415,31 +4415,31 @@ void test_vDHCPProcess_eWaitingAcknowledge_DNSServerOverabundance( void )
/* Add length. */
DHCPOption[ 1 ] = 4;
/* Add the offer byte. */
*( ( uint32_t * ) &DHCPOption[ 2 ] ) = DHCPServerAddress;
memcpy( &DHCPOption[ 2 ], &DHCPServerAddress, sizeof( DHCPServerAddress ) );

DHCPOption += 6;
/* Add Message type code. */
DHCPOption[ 0 ] = dhcpIPv4_SUBNET_MASK_OPTION_CODE;
/* Add length. */
DHCPOption[ 1 ] = 4;
/* Add the offer byte. */
*( ( uint32_t * ) &DHCPOption[ 2 ] ) = ulSubnetMask;
memcpy( &DHCPOption[ 2 ], &ulSubnetMask, sizeof( ulSubnetMask ) );

DHCPOption += 6;
/* Add Message type code. */
DHCPOption[ 0 ] = dhcpIPv4_GATEWAY_OPTION_CODE;
/* Add length. */
DHCPOption[ 1 ] = 4;
/* Add the offer byte. */
*( ( uint32_t * ) &DHCPOption[ 2 ] ) = ulGateway;
memcpy( &DHCPOption[ 2 ], &ulGateway, sizeof( ulGateway ) );

DHCPOption += 6;
/* Add Message type code. */
DHCPOption[ 0 ] = dhcpIPv4_LEASE_TIME_OPTION_CODE;
/* Add length. */
DHCPOption[ 1 ] = 4;
/* Add the offer byte. */
*( ( uint32_t * ) &DHCPOption[ 2 ] ) = ulLeaseTime;
memcpy( &DHCPOption[ 2 ], &ulLeaseTime, sizeof( ulLeaseTime ) );

DHCPOption += 6;
/* Add Message type code. */
Expand Down

0 comments on commit 943ebeb

Please sign in to comment.