From b168c0d2e651aff4e8402208741a77f7ee81e658 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 9 Feb 2021 12:00:13 +0100 Subject: [PATCH] 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 --- tests/suites/test_suite_ccm.function | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/tests/suites/test_suite_ccm.function b/tests/suites/test_suite_ccm.function index faa7e130a..5a3726e07 100644 --- a/tests/suites/test_suite_ccm.function +++ b/tests/suites/test_suite_ccm.function @@ -201,12 +201,11 @@ void mbedtls_ccm_star_encrypt_and_tag( int cipher_id, unsigned char iv[13]; unsigned char result[50]; mbedtls_ccm_context ctx; - size_t i, iv_len, tag_len; + size_t iv_len, tag_len; int ret; mbedtls_ccm_init( &ctx ); - memset( iv, 0x00, sizeof( iv ) ); memset( result, 0x00, sizeof( result ) ); if( sec_level % 4 == 0) @@ -214,12 +213,10 @@ void mbedtls_ccm_star_encrypt_and_tag( int cipher_id, else tag_len = 1 << ( sec_level % 4 + 1); - for( i = 0; i < source_address->len; i++ ) - iv[i] = source_address->x[i]; - - for( i = 0; i < frame_counter->len; i++ ) - iv[source_address->len + i] = frame_counter->x[i]; - + TEST_ASSERT( source_address->len == 8 ); + TEST_ASSERT( frame_counter->len == 4 ); + memcpy( iv, source_address->x, source_address->len ); + memcpy( iv + source_address->len, frame_counter->x, frame_counter->len ); iv[source_address->len + frame_counter->len] = sec_level; iv_len = sizeof( iv ); @@ -254,7 +251,7 @@ void mbedtls_ccm_star_auth_decrypt( int cipher_id, unsigned char iv[13]; unsigned char result[50]; mbedtls_ccm_context ctx; - size_t i, iv_len, tag_len; + size_t iv_len, tag_len; int ret; mbedtls_ccm_init( &ctx ); @@ -267,12 +264,10 @@ void mbedtls_ccm_star_auth_decrypt( int cipher_id, else tag_len = 1 << ( sec_level % 4 + 1); - for( i = 0; i < source_address->len; i++ ) - iv[i] = source_address->x[i]; - - for( i = 0; i < frame_counter->len; i++ ) - iv[source_address->len + i] = frame_counter->x[i]; - + TEST_ASSERT( source_address->len == 8 ); + TEST_ASSERT( frame_counter->len == 4 ); + memcpy( iv, source_address->x, source_address->len ); + memcpy( iv + source_address->len, frame_counter->x, frame_counter->len ); iv[source_address->len + frame_counter->len] = sec_level; iv_len = sizeof( iv );