From ce52d7823c0c8dc4011ba841cda28050ced336fb Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 31 May 2016 14:03:54 +0100 Subject: [PATCH] Address user reported coverity issues. --- include/mbedtls/cipher.h | 1 + library/base64.c | 2 +- library/camellia.c | 50 ++++++++++++++++++------------------ library/cipher.c | 34 ++++++++++++++++-------- library/ecp.c | 4 ++- library/error.c | 2 ++ library/x509_crt.c | 16 ++++++++++-- programs/pkey/dh_client.c | 1 + programs/pkey/dh_genprime.c | 1 + programs/pkey/dh_server.c | 2 ++ programs/pkey/pk_sign.c | 1 + programs/pkey/rsa_decrypt.c | 1 + programs/pkey/rsa_encrypt.c | 1 + programs/pkey/rsa_sign.c | 1 + programs/pkey/rsa_sign_pss.c | 1 + programs/pkey/rsa_verify.c | 1 + programs/test/selftest.c | 3 ++- 17 files changed, 81 insertions(+), 41 deletions(-) diff --git a/include/mbedtls/cipher.h b/include/mbedtls/cipher.h index 70000f5e6..c9675544a 100644 --- a/include/mbedtls/cipher.h +++ b/include/mbedtls/cipher.h @@ -57,6 +57,7 @@ #define MBEDTLS_ERR_CIPHER_INVALID_PADDING -0x6200 /**< Input data contains invalid padding and is rejected. */ #define MBEDTLS_ERR_CIPHER_FULL_BLOCK_EXPECTED -0x6280 /**< Decryption of block requires a full block. */ #define MBEDTLS_ERR_CIPHER_AUTH_FAILED -0x6300 /**< Authentication failed (for AEAD modes). */ +#define MBEDTLS_ERR_CIPHER_INVALID_CONTEXT -0x6380 /**< The context is invalid, eg because it was free()ed. */ #define MBEDTLS_CIPHER_VARIABLE_IV_LEN 0x01 /**< Cipher accepts IVs of variable length */ #define MBEDTLS_CIPHER_VARIABLE_KEY_LEN 0x02 /**< Cipher accepts keys of variable length */ diff --git a/library/base64.c b/library/base64.c index 3432e5fcd..5cb12cba7 100644 --- a/library/base64.c +++ b/library/base64.c @@ -97,7 +97,7 @@ int mbedtls_base64_encode( unsigned char *dst, size_t dlen, size_t *olen, n *= 4; - if( dlen < n + 1 ) + if( ( dlen < n + 1 ) || ( NULL == dst ) ) { *olen = n + 1; return( MBEDTLS_ERR_BASE64_BUFFER_TOO_SMALL ); diff --git a/library/camellia.c b/library/camellia.c index d50513fd0..ac6f96a83 100644 --- a/library/camellia.c +++ b/library/camellia.c @@ -963,38 +963,38 @@ int mbedtls_camellia_self_test( int verbose ) mbedtls_printf( " CAMELLIA-CBC-%3d (%s): ", 128 + u * 64, ( v == MBEDTLS_CAMELLIA_DECRYPT ) ? "dec" : "enc" ); - memcpy( src, camellia_test_cbc_iv, 16 ); - memcpy( dst, camellia_test_cbc_iv, 16 ); - memcpy( key, camellia_test_cbc_key[u], 16 + 8 * u ); - - if( v == MBEDTLS_CAMELLIA_DECRYPT ) { - mbedtls_camellia_setkey_dec( &ctx, key, 128 + u * 64 ); - } else { - mbedtls_camellia_setkey_enc( &ctx, key, 128 + u * 64 ); - } - - for( i = 0; i < CAMELLIA_TESTS_CBC; i++ ) { + memcpy( src, camellia_test_cbc_iv, 16 ); + memcpy( dst, camellia_test_cbc_iv, 16 ); + memcpy( key, camellia_test_cbc_key[u], 16 + 8 * u ); if( v == MBEDTLS_CAMELLIA_DECRYPT ) { - memcpy( iv , src, 16 ); - memcpy( src, camellia_test_cbc_cipher[u][i], 16 ); - memcpy( dst, camellia_test_cbc_plain[i], 16 ); - } else { /* MBEDTLS_CAMELLIA_ENCRYPT */ - memcpy( iv , dst, 16 ); - memcpy( src, camellia_test_cbc_plain[i], 16 ); - memcpy( dst, camellia_test_cbc_cipher[u][i], 16 ); + mbedtls_camellia_setkey_dec( &ctx, key, 128 + u * 64 ); + } else { + mbedtls_camellia_setkey_enc( &ctx, key, 128 + u * 64 ); } - mbedtls_camellia_crypt_cbc( &ctx, v, 16, iv, src, buf ); + for( i = 0; i < CAMELLIA_TESTS_CBC; i++ ) { - if( memcmp( buf, dst, 16 ) != 0 ) - { - if( verbose != 0 ) - mbedtls_printf( "failed\n" ); + if( v == MBEDTLS_CAMELLIA_DECRYPT ) { + memcpy( iv , src, 16 ); + memcpy( src, camellia_test_cbc_cipher[u][i], 16 ); + memcpy( dst, camellia_test_cbc_plain[i], 16 ); + } else { /* MBEDTLS_CAMELLIA_ENCRYPT */ + memcpy( iv , dst, 16 ); + memcpy( src, camellia_test_cbc_plain[i], 16 ); + memcpy( dst, camellia_test_cbc_cipher[u][i], 16 ); + } - return( 1 ); + mbedtls_camellia_crypt_cbc( &ctx, v, 16, iv, src, buf ); + + if( memcmp( buf, dst, 16 ) != 0 ) + { + if( verbose != 0 ) + mbedtls_printf( "failed\n" ); + + return( 1 ); + } } - } if( verbose != 0 ) mbedtls_printf( "passed\n" ); diff --git a/library/cipher.c b/library/cipher.c index 0dc51520f..bbe40eb39 100644 --- a/library/cipher.c +++ b/library/cipher.c @@ -252,6 +252,7 @@ int mbedtls_cipher_update( mbedtls_cipher_context_t *ctx, const unsigned char *i size_t ilen, unsigned char *output, size_t *olen ) { int ret; + size_t block_size = 0; if( NULL == ctx || NULL == ctx->cipher_info || NULL == olen ) { @@ -259,10 +260,11 @@ int mbedtls_cipher_update( mbedtls_cipher_context_t *ctx, const unsigned char *i } *olen = 0; + block_size = mbedtls_cipher_get_block_size( ctx ); if( ctx->cipher_info->mode == MBEDTLS_MODE_ECB ) { - if( ilen != mbedtls_cipher_get_block_size( ctx ) ) + if( ilen != block_size ) return( MBEDTLS_ERR_CIPHER_FULL_BLOCK_EXPECTED ); *olen = ilen; @@ -285,8 +287,13 @@ int mbedtls_cipher_update( mbedtls_cipher_context_t *ctx, const unsigned char *i } #endif + if ( 0 == block_size ) + { + return MBEDTLS_ERR_CIPHER_INVALID_CONTEXT; + } + if( input == output && - ( ctx->unprocessed_len != 0 || ilen % mbedtls_cipher_get_block_size( ctx ) ) ) + ( ctx->unprocessed_len != 0 || ilen % block_size ) ) { return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); } @@ -300,9 +307,9 @@ int mbedtls_cipher_update( mbedtls_cipher_context_t *ctx, const unsigned char *i * If there is not enough data for a full block, cache it. */ if( ( ctx->operation == MBEDTLS_DECRYPT && - ilen + ctx->unprocessed_len <= mbedtls_cipher_get_block_size( ctx ) ) || + ilen + ctx->unprocessed_len <= block_size ) || ( ctx->operation == MBEDTLS_ENCRYPT && - ilen + ctx->unprocessed_len < mbedtls_cipher_get_block_size( ctx ) ) ) + ilen + ctx->unprocessed_len < block_size ) ) { memcpy( &( ctx->unprocessed_data[ctx->unprocessed_len] ), input, ilen ); @@ -314,22 +321,22 @@ int mbedtls_cipher_update( mbedtls_cipher_context_t *ctx, const unsigned char *i /* * Process cached data first */ - if( ctx->unprocessed_len != 0 ) + if( 0 != ctx->unprocessed_len ) { - copy_len = mbedtls_cipher_get_block_size( ctx ) - ctx->unprocessed_len; + copy_len = block_size - ctx->unprocessed_len; memcpy( &( ctx->unprocessed_data[ctx->unprocessed_len] ), input, copy_len ); if( 0 != ( ret = ctx->cipher_info->base->cbc_func( ctx->cipher_ctx, - ctx->operation, mbedtls_cipher_get_block_size( ctx ), ctx->iv, + ctx->operation, block_size, ctx->iv, ctx->unprocessed_data, output ) ) ) { return( ret ); } - *olen += mbedtls_cipher_get_block_size( ctx ); - output += mbedtls_cipher_get_block_size( ctx ); + *olen += block_size; + output += block_size; ctx->unprocessed_len = 0; input += copy_len; @@ -341,9 +348,14 @@ int mbedtls_cipher_update( mbedtls_cipher_context_t *ctx, const unsigned char *i */ if( 0 != ilen ) { - copy_len = ilen % mbedtls_cipher_get_block_size( ctx ); + if( 0 == block_size ) + { + return MBEDTLS_ERR_CIPHER_INVALID_CONTEXT; + } + + copy_len = ilen % block_size; if( copy_len == 0 && ctx->operation == MBEDTLS_DECRYPT ) - copy_len = mbedtls_cipher_get_block_size( ctx ); + copy_len = block_size; memcpy( ctx->unprocessed_data, &( input[ilen - copy_len] ), copy_len ); diff --git a/library/ecp.c b/library/ecp.c index 19bb4882e..f51f2251e 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -1827,7 +1827,9 @@ int mbedtls_ecp_gen_keypair_base( mbedtls_ecp_group *grp, /* [M225] page 5 */ size_t b; - MBEDTLS_MPI_CHK( mbedtls_mpi_fill_random( d, n_size, f_rng, p_rng ) ); + do { + MBEDTLS_MPI_CHK( mbedtls_mpi_fill_random( d, n_size, f_rng, p_rng ) ); + } while( mbedtls_mpi_bitlen( d ) == 0); /* Make sure the most significant bit is nbits */ b = mbedtls_mpi_bitlen( d ) - 1; /* mbedtls_mpi_bitlen is one-based */ diff --git a/library/error.c b/library/error.c index 4718b514d..4bd15bfee 100644 --- a/library/error.c +++ b/library/error.c @@ -183,6 +183,8 @@ void mbedtls_strerror( int ret, char *buf, size_t buflen ) mbedtls_snprintf( buf, buflen, "CIPHER - Decryption of block requires a full block" ); if( use_ret == -(MBEDTLS_ERR_CIPHER_AUTH_FAILED) ) mbedtls_snprintf( buf, buflen, "CIPHER - Authentication failed (for AEAD modes)" ); + if( use_ret == -(MBEDTLS_ERR_CIPHER_INVALID_CONTEXT) ) + mbedtls_snprintf( buf, buflen, "CIPHER - The context is invalid, eg because it was free()ed" ); #endif /* MBEDTLS_CIPHER_C */ #if defined(MBEDTLS_DHM_C) diff --git a/library/x509_crt.c b/library/x509_crt.c index c3adf7c86..af6c2a4a5 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -970,7 +970,9 @@ int mbedtls_x509_crt_parse_der( mbedtls_x509_crt *chain, const unsigned char *bu int mbedtls_x509_crt_parse( mbedtls_x509_crt *chain, const unsigned char *buf, size_t buflen ) { int success = 0, first_error = 0, total_failed = 0; +#if defined(MBEDTLS_PEM_PARSE_C) int buf_format = MBEDTLS_X509_FORMAT_DER; +#endif /* * Check for valid input @@ -988,10 +990,12 @@ int mbedtls_x509_crt_parse( mbedtls_x509_crt *chain, const unsigned char *buf, s { buf_format = MBEDTLS_X509_FORMAT_PEM; } -#endif if( buf_format == MBEDTLS_X509_FORMAT_DER ) return mbedtls_x509_crt_parse_der( chain, buf, buflen ); +#else + return mbedtls_x509_crt_parse_der( chain, buf, buflen ); +#endif #if defined(MBEDTLS_PEM_PARSE_C) if( buf_format == MBEDTLS_X509_FORMAT_PEM ) @@ -1064,7 +1068,6 @@ int mbedtls_x509_crt_parse( mbedtls_x509_crt *chain, const unsigned char *buf, s success = 1; } } -#endif /* MBEDTLS_PEM_PARSE_C */ if( success ) return( total_failed ); @@ -1072,6 +1075,7 @@ int mbedtls_x509_crt_parse( mbedtls_x509_crt *chain, const unsigned char *buf, s return( first_error ); else return( MBEDTLS_ERR_X509_CERT_UNKNOWN_FORMAT ); +#endif /* MBEDTLS_PEM_PARSE_C */ } #if defined(MBEDTLS_FS_IO) @@ -1353,6 +1357,14 @@ int mbedtls_x509_crt_info( char *buf, size_t size, const char *prefix, p = buf; n = size; + if( NULL == crt ) + { + ret = mbedtls_snprintf( p, n, "\nCertificate is uninitialised!\n" ); + MBEDTLS_X509_SAFE_SNPRINTF; + + return( (int) ( size - n ) ); + } + ret = mbedtls_snprintf( p, n, "%scert. version : %d\n", prefix, crt->version ); MBEDTLS_X509_SAFE_SNPRINTF; diff --git a/programs/pkey/dh_client.c b/programs/pkey/dh_client.c index 230bf4d7c..8ebf34a77 100644 --- a/programs/pkey/dh_client.c +++ b/programs/pkey/dh_client.c @@ -125,6 +125,7 @@ int main( void ) ( ret = mbedtls_mpi_read_file( &rsa.E, 16, f ) ) != 0 ) { mbedtls_printf( " failed\n ! mbedtls_mpi_read_file returned %d\n\n", ret ); + fclose( f ); goto exit; } diff --git a/programs/pkey/dh_genprime.c b/programs/pkey/dh_genprime.c index d30c73bf7..072fe138f 100644 --- a/programs/pkey/dh_genprime.c +++ b/programs/pkey/dh_genprime.c @@ -172,6 +172,7 @@ int main( int argc, char **argv ) ( ret = mbedtls_mpi_write_file( "G = ", &G, 16, fout ) != 0 ) ) { mbedtls_printf( " failed\n ! mbedtls_mpi_write_file returned %d\n\n", ret ); + fclose( fout ); goto exit; } diff --git a/programs/pkey/dh_server.c b/programs/pkey/dh_server.c index cb156f79b..7eef845df 100644 --- a/programs/pkey/dh_server.c +++ b/programs/pkey/dh_server.c @@ -132,6 +132,7 @@ int main( void ) ( ret = mbedtls_mpi_read_file( &rsa.QP, 16, f ) ) != 0 ) { mbedtls_printf( " failed\n ! mbedtls_mpi_read_file returned %d\n\n", ret ); + fclose( f ); goto exit; } @@ -157,6 +158,7 @@ int main( void ) mbedtls_mpi_read_file( &dhm.G, 16, f ) != 0 ) { mbedtls_printf( " failed\n ! Invalid DH parameter file\n\n" ); + fclose( f ); goto exit; } diff --git a/programs/pkey/pk_sign.c b/programs/pkey/pk_sign.c index 322e8aff0..daf08a905 100644 --- a/programs/pkey/pk_sign.c +++ b/programs/pkey/pk_sign.c @@ -142,6 +142,7 @@ int main( int argc, char *argv[] ) if( fwrite( buf, 1, olen, f ) != olen ) { mbedtls_printf( "failed\n ! fwrite failed\n\n" ); + fclose( f ); goto exit; } diff --git a/programs/pkey/rsa_decrypt.c b/programs/pkey/rsa_decrypt.c index 94431e0ce..194f2de40 100644 --- a/programs/pkey/rsa_decrypt.c +++ b/programs/pkey/rsa_decrypt.c @@ -116,6 +116,7 @@ int main( int argc, char *argv[] ) ( ret = mbedtls_mpi_read_file( &rsa.QP, 16, f ) ) != 0 ) { mbedtls_printf( " failed\n ! mbedtls_mpi_read_file returned %d\n\n", ret ); + fclose( f ); goto exit; } diff --git a/programs/pkey/rsa_encrypt.c b/programs/pkey/rsa_encrypt.c index 796343f1b..d3e415a2b 100644 --- a/programs/pkey/rsa_encrypt.c +++ b/programs/pkey/rsa_encrypt.c @@ -110,6 +110,7 @@ int main( int argc, char *argv[] ) ( ret = mbedtls_mpi_read_file( &rsa.E, 16, f ) ) != 0 ) { mbedtls_printf( " failed\n ! mbedtls_mpi_read_file returned %d\n\n", ret ); + fclose( f ); goto exit; } diff --git a/programs/pkey/rsa_sign.c b/programs/pkey/rsa_sign.c index e897c6519..da723412b 100644 --- a/programs/pkey/rsa_sign.c +++ b/programs/pkey/rsa_sign.c @@ -98,6 +98,7 @@ int main( int argc, char *argv[] ) ( ret = mbedtls_mpi_read_file( &rsa.QP, 16, f ) ) != 0 ) { mbedtls_printf( " failed\n ! mbedtls_mpi_read_file returned %d\n\n", ret ); + fclose( f ); goto exit; } diff --git a/programs/pkey/rsa_sign_pss.c b/programs/pkey/rsa_sign_pss.c index c045a04c1..7b6f14dd8 100644 --- a/programs/pkey/rsa_sign_pss.c +++ b/programs/pkey/rsa_sign_pss.c @@ -153,6 +153,7 @@ int main( int argc, char *argv[] ) if( fwrite( buf, 1, olen, f ) != olen ) { mbedtls_printf( "failed\n ! fwrite failed\n\n" ); + fclose( f ); goto exit; } diff --git a/programs/pkey/rsa_verify.c b/programs/pkey/rsa_verify.c index ade36dc83..8bc51d85e 100644 --- a/programs/pkey/rsa_verify.c +++ b/programs/pkey/rsa_verify.c @@ -89,6 +89,7 @@ int main( int argc, char *argv[] ) ( ret = mbedtls_mpi_read_file( &rsa.E, 16, f ) ) != 0 ) { mbedtls_printf( " failed\n ! mbedtls_mpi_read_file returned %d\n\n", ret ); + fclose( f ); goto exit; } diff --git a/programs/test/selftest.c b/programs/test/selftest.c index 6ca07bba2..7698b629f 100644 --- a/programs/test/selftest.c +++ b/programs/test/selftest.c @@ -397,6 +397,7 @@ int main( int argc, char *argv[] ) if( suites_failed > 0) mbedtls_exit( MBEDTLS_EXIT_FAILURE ); - mbedtls_exit( MBEDTLS_EXIT_SUCCESS ); + /* return() is here to prevent compiler warnings */ + return( 0 ); }