From dc7b15c11f5818c380d2196075435e61995c4dfc Mon Sep 17 00:00:00 2001
From: Janos Follath <janos.follath@arm.com>
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 70000f5e61..c9675544a9 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 3432e5fcd7..5cb12cba75 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 d50513fd07..ac6f96a83a 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 0dc51520fa..bbe40eb395 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 19bb4882e7..f51f2251ed 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 4718b514d2..4bd15bfee4 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 c3adf7c864..af6c2a4a53 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 230bf4d7c8..8ebf34a77b 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 d30c73bf7f..072fe138f5 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 cb156f79b3..7eef845dfb 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 322e8aff0c..daf08a9055 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 94431e0cec..194f2de404 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 796343f1b3..d3e415a2b0 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 e897c65197..da723412b3 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 c045a04c1c..7b6f14dd8e 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 ade36dc830..8bc51d85e6 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 6ca07bba23..7698b629fd 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 );
}