From 6a54336897e5150b95c64bf1eea122cdd9556a9c Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Tue, 17 Jan 2017 23:04:22 +0000 Subject: [PATCH] Fix integer overflows in buffer bound checks Fix potential integer overflows in the following functions: * mbedtls_md2_update() to be bypassed and cause * mbedtls_cipher_update() * mbedtls_ctr_drbg_reseed() This overflows would mainly be exploitable in 32-bit systems and could cause buffer bound checks to be bypassed. --- ChangeLog | 6 ++++++ library/cipher.c | 4 ++-- library/ctr_drbg.c | 3 ++- library/md2.c | 2 +- tests/suites/test_suite_ctr_drbg.function | 5 +++++ 5 files changed, 16 insertions(+), 4 deletions(-) diff --git a/ChangeLog b/ChangeLog index f24186b36..fc240c305 100644 --- a/ChangeLog +++ b/ChangeLog @@ -21,6 +21,12 @@ Bugfix * Fixed multiple buffer overreads in mbedtls_pem_read_buffer() when parsing the input string in PEM format to extract the different components. Found by Eyal Itkin. + * Fixed potential arithmetic overflow in mbedtls_ctr_drbg_reseed() that could + cause buffer bound checks to be bypassed. Found by Eyal Itkin. + * Fixed potential arithmetic overflows in mbedtls_cipher_update() that could + cause buffer bound checks to be bypassed. Found by Eyal Itkin. + * Fixed potential arithmetic overflow in mbedtls_md2_update() that could + cause buffer bound checks to be bypassed. Found by Eyal Itkin. = mbed TLS 2.4.1 branch released 2016-12-13 diff --git a/library/cipher.c b/library/cipher.c index a88343869..e9e0b223e 100644 --- a/library/cipher.c +++ b/library/cipher.c @@ -326,9 +326,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 <= block_size ) || + ilen <= block_size - ctx->unprocessed_len ) || ( ctx->operation == MBEDTLS_ENCRYPT && - ilen + ctx->unprocessed_len < block_size ) ) + ilen < block_size - ctx->unprocessed_len ) ) { memcpy( &( ctx->unprocessed_data[ctx->unprocessed_len] ), input, ilen ); diff --git a/library/ctr_drbg.c b/library/ctr_drbg.c index 386f8adb0..55612c7fc 100644 --- a/library/ctr_drbg.c +++ b/library/ctr_drbg.c @@ -290,7 +290,8 @@ int mbedtls_ctr_drbg_reseed( mbedtls_ctr_drbg_context *ctx, unsigned char seed[MBEDTLS_CTR_DRBG_MAX_SEED_INPUT]; size_t seedlen = 0; - if( ctx->entropy_len + len > MBEDTLS_CTR_DRBG_MAX_SEED_INPUT ) + if( ctx->entropy_len > MBEDTLS_CTR_DRBG_MAX_SEED_INPUT || + len > MBEDTLS_CTR_DRBG_MAX_SEED_INPUT - ctx->entropy_len ) return( MBEDTLS_ERR_CTR_DRBG_INPUT_TOO_BIG ); memset( seed, 0, MBEDTLS_CTR_DRBG_MAX_SEED_INPUT ); diff --git a/library/md2.c b/library/md2.c index 897670131..95cbcce65 100644 --- a/library/md2.c +++ b/library/md2.c @@ -158,7 +158,7 @@ void mbedtls_md2_update( mbedtls_md2_context *ctx, const unsigned char *input, s while( ilen > 0 ) { - if( ctx->left + ilen > 16 ) + if( ilen > 16 - ctx->left ) fill = 16 - ctx->left; else fill = ilen; diff --git a/tests/suites/test_suite_ctr_drbg.function b/tests/suites/test_suite_ctr_drbg.function index 3acfb8bae..883cfe08e 100644 --- a/tests/suites/test_suite_ctr_drbg.function +++ b/tests/suites/test_suite_ctr_drbg.function @@ -39,6 +39,11 @@ void ctr_drbg_special_behaviours( ) TEST_ASSERT( mbedtls_ctr_drbg_reseed( &ctx, additional, MBEDTLS_CTR_DRBG_MAX_SEED_INPUT + 1 ) == MBEDTLS_ERR_CTR_DRBG_INPUT_TOO_BIG ); + + mbedtls_ctr_drbg_set_entropy_len( &ctx, ~0 ); + TEST_ASSERT( mbedtls_ctr_drbg_reseed( &ctx, additional, + MBEDTLS_CTR_DRBG_MAX_SEED_INPUT ) == + MBEDTLS_ERR_CTR_DRBG_INPUT_TOO_BIG ); exit: mbedtls_ctr_drbg_free( &ctx ); }