From a44e7d854e1fc720ed5ac7f64116767ce1a1d3b2 Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Tue, 17 Jan 2017 23:04:22 +0000 Subject: [PATCH 1/3] 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 | 10 ++++++++++ library/cipher.c | 4 ++-- library/ctr_drbg.c | 3 ++- library/md2.c | 2 +- tests/suites/test_suite_ctr_drbg.function | 5 +++++ 5 files changed, 20 insertions(+), 4 deletions(-) diff --git a/ChangeLog b/ChangeLog index 0a857ba76..b82ab0a47 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,15 @@ mbed TLS ChangeLog (Sorted per branch, date) += mbed TLS 2.x.x branch released xxxx-xx-xx + +Bugfix + * 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 Changes 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 ); } From 5718ebf45bbd3350e935cfde6b28c4ee6cc6ff88 Mon Sep 17 00:00:00 2001 From: Andres AG Date: Wed, 18 Jan 2017 17:21:03 +0000 Subject: [PATCH 2/3] Fix integer overflow mbedtls_base64_decode() Fix potential integer overflows in the function mbedtls_base64_decode(). This overflow would mainly be exploitable in 32-bit systems and could cause buffer bound checks to be bypassed. --- ChangeLog | 6 ++++++ library/base64.c | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 0a857ba76..d68082bea 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,11 @@ mbed TLS ChangeLog (Sorted per branch, date) += mbed TLS 2.x.x branch released xxxx-xx-xx + +Bugfix + * Fixed potential arithmetic overflow in mbedtls_base64_decode() that could + cause buffer bound checks to be bypassed. Found by Eyal Itkin. + = mbed TLS 2.4.1 branch released 2016-12-13 Changes diff --git a/library/base64.c b/library/base64.c index 5cb12cba7..305afc57b 100644 --- a/library/base64.c +++ b/library/base64.c @@ -192,7 +192,7 @@ int mbedtls_base64_decode( unsigned char *dst, size_t dlen, size_t *olen, return( 0 ); } - n = ( ( n * 6 ) + 7 ) >> 3; + n = ( 6 * ( n >> 3 ) ) + ( ( 6 * ( n & 0x7 ) + 7 ) >> 3 ); n -= j; if( dst == NULL || dlen < n ) From cd8f1476b4ca7967d54240965546102fdfb1be9e Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Thu, 2 Feb 2017 08:46:53 +0000 Subject: [PATCH 3/3] Add comment to integer overflow fix in base64.c Adds clarifying comment to the integer overflow fix in base64.c --- library/base64.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/library/base64.c b/library/base64.c index 305afc57b..f06b57b31 100644 --- a/library/base64.c +++ b/library/base64.c @@ -192,6 +192,10 @@ int mbedtls_base64_decode( unsigned char *dst, size_t dlen, size_t *olen, return( 0 ); } + /* The following expression is to calculate the following formula without + * risk of integer overflow in n: + * n = ( ( n * 6 ) + 7 ) >> 3; + */ n = ( 6 * ( n >> 3 ) ) + ( ( 6 * ( n & 0x7 ) + 7 ) >> 3 ); n -= j;