From 3a3b161e9629ffcece3909c04a7369a49e8c44cc Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 21 Jan 2020 16:56:03 +0100 Subject: [PATCH 1/5] Add missing return code check on call to mbedtls_md() --- library/x509write_csr.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/library/x509write_csr.c b/library/x509write_csr.c index b65a11c6a..7406a9754 100644 --- a/library/x509write_csr.c +++ b/library/x509write_csr.c @@ -226,7 +226,9 @@ int mbedtls_x509write_csr_der( mbedtls_x509write_csr *ctx, unsigned char *buf, s /* * Prepare signature */ - mbedtls_md( mbedtls_md_info_from_type( ctx->md_alg ), c, len, hash ); + ret = mbedtls_md( mbedtls_md_info_from_type( ctx->md_alg ), c, len, hash ); + if( ret != 0 ) + return( ret ); if( ( ret = mbedtls_pk_sign( ctx->key, ctx->md_alg, hash, 0, sig, &sig_len, f_rng, p_rng ) ) != 0 ) From 42e4f6b706fafbd3f172f2e0c92bd3f238e2ba41 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 22 Jan 2020 19:04:19 +0100 Subject: [PATCH 2/5] Add changelog entry for the unchecked mbedtls_md call --- ChangeLog | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ChangeLog b/ChangeLog index b8dc65c33..6a1c6377a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,10 @@ mbed TLS ChangeLog (Sorted per branch, date) += mbed TLS 2.16.5 branch released xxxx-xx-xx + +Bugfix + * Fix an unchecked call to mbedtls_md() in the x509write module. + = mbed TLS 2.16.4 branch released 2020-01-15 Security From 83a5672ae13de8358370e0bbaa6fcc71d93d2e8c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 21 Jan 2020 15:02:14 +0100 Subject: [PATCH 3/5] Remove redundant block_size validity check Check the value only once, as soon as we've obtained it. --- library/cipher.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/library/cipher.c b/library/cipher.c index 273997577..8d010b59a 100644 --- a/library/cipher.c +++ b/library/cipher.c @@ -361,6 +361,10 @@ int mbedtls_cipher_update( mbedtls_cipher_context_t *ctx, const unsigned char *i *olen = 0; block_size = mbedtls_cipher_get_block_size( ctx ); + if ( 0 == block_size ) + { + return( MBEDTLS_ERR_CIPHER_INVALID_CONTEXT ); + } if( ctx->cipher_info->mode == MBEDTLS_MODE_ECB ) { @@ -396,11 +400,6 @@ 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 % block_size ) ) { @@ -459,11 +458,6 @@ int mbedtls_cipher_update( mbedtls_cipher_context_t *ctx, const unsigned char *i */ if( 0 != ilen ) { - if( 0 == block_size ) - { - return( MBEDTLS_ERR_CIPHER_INVALID_CONTEXT ); - } - /* Encryption: only cache partial blocks * Decryption w/ padding: always keep at least one whole block * Decryption w/o padding: only cache partial blocks From 1a30fbbd3b6ad74bfb9a13a24ba11c07aab9dd5e Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 21 Jan 2020 16:30:53 +0100 Subject: [PATCH 4/5] Check that mbedtls_mpi_grow succeeds --- tests/suites/test_suite_mpi.function | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/suites/test_suite_mpi.function b/tests/suites/test_suite_mpi.function index 97c338b85..bebca5a0a 100644 --- a/tests/suites/test_suite_mpi.function +++ b/tests/suites/test_suite_mpi.function @@ -550,8 +550,8 @@ void mbedtls_mpi_lt_mpi_ct( int size_X, char * input_X, TEST_ASSERT( mbedtls_mpi_read_string( &X, 16, input_X ) == 0 ); TEST_ASSERT( mbedtls_mpi_read_string( &Y, 16, input_Y ) == 0 ); - mbedtls_mpi_grow( &X, size_X ); - mbedtls_mpi_grow( &Y, size_Y ); + TEST_ASSERT( mbedtls_mpi_grow( &X, size_X ) == 0 ); + TEST_ASSERT( mbedtls_mpi_grow( &Y, size_Y ) == 0 ); TEST_ASSERT( mbedtls_mpi_lt_mpi_ct( &X, &Y, &ret ) == input_err ); if( input_err == 0 ) From 75aab5276f9490b6b0220daee23ff4423a59495d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 21 Jan 2020 16:52:08 +0100 Subject: [PATCH 5/5] Add missing return code check on calls to mbedtls_md() --- tests/suites/test_suite_ecdsa.function | 4 +++- tests/suites/test_suite_pk.function | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/suites/test_suite_ecdsa.function b/tests/suites/test_suite_ecdsa.function index 0e7283bc7..2844beaa5 100644 --- a/tests/suites/test_suite_ecdsa.function +++ b/tests/suites/test_suite_ecdsa.function @@ -527,7 +527,9 @@ void ecdsa_write_restart( int id, char *d_str, int md_alg, TEST_ASSERT( md_info != NULL ); hlen = mbedtls_md_get_size( md_info ); - mbedtls_md( md_info, (const unsigned char *) msg, strlen( msg ), hash ); + TEST_ASSERT( mbedtls_md( md_info, + (const unsigned char *) msg, strlen( msg ), + hash ) == 0 ); mbedtls_ecp_set_max_ops( max_ops ); diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index 342405e5d..b57fe19fa 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -712,7 +712,9 @@ void pk_sign_verify_restart( int pk_type, int grp_id, char *d_str, TEST_ASSERT( md_info != NULL ); hlen = mbedtls_md_get_size( md_info ); - mbedtls_md( md_info, (const unsigned char *) msg, strlen( msg ), hash ); + TEST_ASSERT( mbedtls_md( md_info, + (const unsigned char *) msg, strlen( msg ), + hash ) == 0 ); mbedtls_ecp_set_max_ops( max_ops );