Remove trail check in the generate_random test

The test function generate_random allocated a few extra bytes after
the expected output and checked that these extra bytes were not
overwritten. Memory sanity checks such as AddressSanitizer and
Valgrind already detect this kind of buffer overflow, so having this
test in our code was actually redundant. Remove it.

This has the benefit of not triggering a build error with GCC
(observed with 7.5.0 and 9.3.0) when ASan+UBSan is enabled: with the
previous code using trail, GCC complained about an excessively large
value passed to calloc(), which was (size_t)(-sizeof(trail)).
Thus this commit fixes #4122.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
This commit is contained in:
Gilles Peskine 2021-02-08 19:50:26 +01:00
parent a0832d47f7
commit 9189202156

View File

@ -5543,7 +5543,6 @@ exit:
void generate_random( int bytes_arg ) void generate_random( int bytes_arg )
{ {
size_t bytes = bytes_arg; size_t bytes = bytes_arg;
const unsigned char trail[] = "don't overwrite me";
unsigned char *output = NULL; unsigned char *output = NULL;
unsigned char *changed = NULL; unsigned char *changed = NULL;
size_t i; size_t i;
@ -5551,9 +5550,8 @@ void generate_random( int bytes_arg )
TEST_ASSERT( bytes_arg >= 0 ); TEST_ASSERT( bytes_arg >= 0 );
ASSERT_ALLOC( output, bytes + sizeof( trail ) ); ASSERT_ALLOC( output, bytes );
ASSERT_ALLOC( changed, bytes ); ASSERT_ALLOC( changed, bytes );
memcpy( output + bytes, trail, sizeof( trail ) );
PSA_ASSERT( psa_crypto_init( ) ); PSA_ASSERT( psa_crypto_init( ) );
@ -5566,10 +5564,6 @@ void generate_random( int bytes_arg )
memset( output, 0, bytes ); memset( output, 0, bytes );
PSA_ASSERT( psa_generate_random( output, bytes ) ); PSA_ASSERT( psa_generate_random( output, bytes ) );
/* Check that no more than bytes have been overwritten */
ASSERT_COMPARE( output + bytes, sizeof( trail ),
trail, sizeof( trail ) );
for( i = 0; i < bytes; i++ ) for( i = 0; i < bytes; i++ )
{ {
if( output[i] != 0 ) if( output[i] != 0 )