From 3d283787344afd3a1172b280f70c6cfab41c2f65 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 10 Dec 2021 14:25:45 +0100 Subject: [PATCH 1/2] Check return values in more places Selective replacement of ``` ^\( *\)\(mbedtls_\(md\|cipher\)_[A-Z_a-z0-9]+\)\((.*)\); ``` by ``` \1if( \2\4 != 0 ) \1{ \1 mbedtls_fprintf( stderr, "\2() returned error\\n" ); \1 goto exit; \1} ``` Signed-off-by: Gilles Peskine --- programs/aes/crypt_and_hash.c | 72 +++++++++++++++++++++++++++++------ 1 file changed, 60 insertions(+), 12 deletions(-) diff --git a/programs/aes/crypt_and_hash.c b/programs/aes/crypt_and_hash.c index bdc5c04d2..18bdf6cd2 100644 --- a/programs/aes/crypt_and_hash.c +++ b/programs/aes/crypt_and_hash.c @@ -367,7 +367,11 @@ int main( int argc, char *argv[] ) goto exit; } - mbedtls_md_hmac_starts( &md_ctx, digest, 32 ); + if( mbedtls_md_hmac_starts( &md_ctx, digest, 32 ) != 0 ) + { + mbedtls_fprintf( stderr, "mbedtls_md_hmac_starts() returned error\n" ); + goto exit; + } /* * Encrypt and write the ciphertext. @@ -389,7 +393,11 @@ int main( int argc, char *argv[] ) goto exit; } - mbedtls_md_hmac_update( &md_ctx, output, olen ); + if( mbedtls_md_hmac_update( &md_ctx, output, olen ) != 0 ) + { + mbedtls_fprintf( stderr, "mbedtls_md_hmac_update() returned error\n" ); + goto exit; + } if( fwrite( output, 1, olen, fout ) != olen ) { @@ -403,7 +411,11 @@ int main( int argc, char *argv[] ) mbedtls_fprintf( stderr, "mbedtls_cipher_finish() returned error\n" ); goto exit; } - mbedtls_md_hmac_update( &md_ctx, output, olen ); + if( mbedtls_md_hmac_update( &md_ctx, output, olen ) != 0 ) + { + mbedtls_fprintf( stderr, "mbedtls_md_hmac_update() returned error\n" ); + goto exit; + } if( fwrite( output, 1, olen, fout ) != olen ) { @@ -414,7 +426,11 @@ int main( int argc, char *argv[] ) /* * Finally write the HMAC. */ - mbedtls_md_hmac_finish( &md_ctx, digest ); + if( mbedtls_md_hmac_finish( &md_ctx, digest ) != 0 ) + { + mbedtls_fprintf( stderr, "mbedtls_md_hmac_finish() returned error\n" ); + goto exit; + } if( fwrite( digest, 1, mbedtls_md_get_size( md_info ), fout ) != mbedtls_md_get_size( md_info ) ) { @@ -483,10 +499,26 @@ int main( int argc, char *argv[] ) for( i = 0; i < 8192; i++ ) { - mbedtls_md_starts( &md_ctx ); - mbedtls_md_update( &md_ctx, digest, 32 ); - mbedtls_md_update( &md_ctx, key, keylen ); - mbedtls_md_finish( &md_ctx, digest ); + if( mbedtls_md_starts( &md_ctx ) != 0 ) + { + mbedtls_fprintf( stderr, "mbedtls_md_starts() returned error\n" ); + goto exit; + } + if( mbedtls_md_update( &md_ctx, digest, 32 ) != 0 ) + { + mbedtls_fprintf( stderr, "mbedtls_md_update() returned error\n" ); + goto exit; + } + if( mbedtls_md_update( &md_ctx, key, keylen ) != 0 ) + { + mbedtls_fprintf( stderr, "mbedtls_md_update() returned error\n" ); + goto exit; + } + if( mbedtls_md_finish( &md_ctx, digest ) != 0 ) + { + mbedtls_fprintf( stderr, "mbedtls_md_finish() returned error\n" ); + goto exit; + } } if( mbedtls_cipher_setkey( &cipher_ctx, digest, cipher_info->key_bitlen, @@ -508,7 +540,11 @@ int main( int argc, char *argv[] ) goto exit; } - mbedtls_md_hmac_starts( &md_ctx, digest, 32 ); + if( mbedtls_md_hmac_starts( &md_ctx, digest, 32 ) != 0 ) + { + mbedtls_fprintf( stderr, "mbedtls_md_hmac_starts() returned error\n" ); + goto exit; + } /* * Decrypt and write the plaintext. @@ -525,7 +561,11 @@ int main( int argc, char *argv[] ) goto exit; } - mbedtls_md_hmac_update( &md_ctx, buffer, ilen ); + if( mbedtls_md_hmac_update( &md_ctx, buffer, ilen ) != 0 ) + { + mbedtls_fprintf( stderr, "mbedtls_md_hmac_update() returned error\n" ); + goto exit; + } if( mbedtls_cipher_update( &cipher_ctx, buffer, ilen, output, &olen ) != 0 ) { @@ -543,7 +583,11 @@ int main( int argc, char *argv[] ) /* * Verify the message authentication code. */ - mbedtls_md_hmac_finish( &md_ctx, digest ); + if( mbedtls_md_hmac_finish( &md_ctx, digest ) != 0 ) + { + mbedtls_fprintf( stderr, "mbedtls_md_hmac_finish() returned error\n" ); + goto exit; + } if( fread( buffer, 1, mbedtls_md_get_size( md_info ), fin ) != mbedtls_md_get_size( md_info ) ) { @@ -566,7 +610,11 @@ int main( int argc, char *argv[] ) /* * Write the final block of data */ - mbedtls_cipher_finish( &cipher_ctx, output, &olen ); + if( mbedtls_cipher_finish( &cipher_ctx, output, &olen ) != 0 ) + { + mbedtls_fprintf( stderr, "mbedtls_cipher_finish() returned error\n" ); + goto exit; + } if( fwrite( output, 1, olen, fout ) != olen ) { From 3fc0d304476ce4feee06a92d856ba889fd4f5d2a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 10 Dec 2021 14:28:31 +0100 Subject: [PATCH 2/2] Don't fail until everything is initialized Can't call mbedtls_cipher_free(&invalid_ctx) in cleanup if mbedtls_cipher_init(&invalid_ctx) hasn't been called. Signed-off-by: Gilles Peskine --- tests/suites/test_suite_cipher.function | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/suites/test_suite_cipher.function b/tests/suites/test_suite_cipher.function index b50422e39..b0b1629a8 100644 --- a/tests/suites/test_suite_cipher.function +++ b/tests/suites/test_suite_cipher.function @@ -102,9 +102,10 @@ void cipher_invalid_param_unconditional( ) (void)valid_mode; /* In some configurations this is unused */ mbedtls_cipher_init( &valid_ctx ); - TEST_ASSERT( mbedtls_cipher_setup( &valid_ctx, valid_info ) == 0 ); mbedtls_cipher_init( &invalid_ctx ); + TEST_ASSERT( mbedtls_cipher_setup( &valid_ctx, valid_info ) == 0 ); + /* mbedtls_cipher_setup() */ TEST_ASSERT( mbedtls_cipher_setup( &valid_ctx, NULL ) == MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA );