More robust code to set the IV

Check that the source address and the frame counter have the expected
length. Otherwise, if the test data was invalid, the test code could
build nonsensical inputs, potentially overflowing the iv buffer.

The primary benefit of this change is that it also silences a warning
from compiling with `gcc-10 -O3` (observed with GCC 10.2.0 on
Linux/amd64). GCC unrolled the loops and complained about a buffer
overflow with warnings like:
```
suites/test_suite_ccm.function: In function 'test_mbedtls_ccm_star_auth_decrypt':
suites/test_suite_ccm.function:271:15: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=]
  271 |         iv[i] = source_address->x[i];
      |         ~~~~~~^~~~~~~~~~~~~~~~~~~~~~
suites/test_suite_ccm.function:254:19: note: at offset [13, 14] to object 'iv' with size 13 declared here
  254 |     unsigned char iv[13];
```
Just using memcpy instead of loops bypasses this warnings. The added
checks are a bonus.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
This commit is contained in:
Gilles Peskine 2021-02-09 12:00:13 +01:00
parent 47e4035e98
commit e8d7e6c6e4

View File

@ -200,12 +200,11 @@ void mbedtls_ccm_star_encrypt_and_tag( int cipher_id,
unsigned char iv[13]; unsigned char iv[13];
unsigned char result[50]; unsigned char result[50];
mbedtls_ccm_context ctx; mbedtls_ccm_context ctx;
size_t i, iv_len, tag_len; size_t iv_len, tag_len;
int ret; int ret;
mbedtls_ccm_init( &ctx ); mbedtls_ccm_init( &ctx );
memset( iv, 0x00, sizeof( iv ) );
memset( result, 0x00, sizeof( result ) ); memset( result, 0x00, sizeof( result ) );
if( sec_level % 4 == 0) if( sec_level % 4 == 0)
@ -213,12 +212,10 @@ void mbedtls_ccm_star_encrypt_and_tag( int cipher_id,
else else
tag_len = 1 << ( sec_level % 4 + 1); tag_len = 1 << ( sec_level % 4 + 1);
for( i = 0; i < source_address->len; i++ ) TEST_ASSERT( source_address->len == 8 );
iv[i] = source_address->x[i]; TEST_ASSERT( frame_counter->len == 4 );
memcpy( iv, source_address->x, source_address->len );
for( i = 0; i < frame_counter->len; i++ ) memcpy( iv + source_address->len, frame_counter->x, frame_counter->len );
iv[source_address->len + i] = frame_counter->x[i];
iv[source_address->len + frame_counter->len] = sec_level; iv[source_address->len + frame_counter->len] = sec_level;
iv_len = sizeof( iv ); iv_len = sizeof( iv );
@ -253,7 +250,7 @@ void mbedtls_ccm_star_auth_decrypt( int cipher_id,
unsigned char iv[13]; unsigned char iv[13];
unsigned char result[50]; unsigned char result[50];
mbedtls_ccm_context ctx; mbedtls_ccm_context ctx;
size_t i, iv_len, tag_len; size_t iv_len, tag_len;
int ret; int ret;
mbedtls_ccm_init( &ctx ); mbedtls_ccm_init( &ctx );
@ -266,12 +263,10 @@ void mbedtls_ccm_star_auth_decrypt( int cipher_id,
else else
tag_len = 1 << ( sec_level % 4 + 1); tag_len = 1 << ( sec_level % 4 + 1);
for( i = 0; i < source_address->len; i++ ) TEST_ASSERT( source_address->len == 8 );
iv[i] = source_address->x[i]; TEST_ASSERT( frame_counter->len == 4 );
memcpy( iv, source_address->x, source_address->len );
for( i = 0; i < frame_counter->len; i++ ) memcpy( iv + source_address->len, frame_counter->x, frame_counter->len );
iv[source_address->len + i] = frame_counter->x[i];
iv[source_address->len + frame_counter->len] = sec_level; iv[source_address->len + frame_counter->len] = sec_level;
iv_len = sizeof( iv ); iv_len = sizeof( iv );