Address user reported coverity issues.

gilles-peskine-arm · Jun 9, 2016 · 98e28a7 · 98e28a7
1 parent f991128
commit 98e28a7
Show file tree

Hide file tree

Showing 17 changed files with 81 additions and 41 deletions.
diff --git 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
@@ -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
@@ -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
@@ -252,17 +252,19 @@ 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 )
     {
         return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA );
     }
 
     *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
@@ -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
@@ -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
@@ -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,14 +1068,14 @@ 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 );
     else if( first_error )
         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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 );
 }