From d3cbc15951e713f5ca3d7eff916330c4eaa4b95a Mon Sep 17 00:00:00 2001 From: Andres AG Date: Mon, 24 Oct 2016 11:23:36 +0100 Subject: [PATCH 01/17] Fix buffer overreads in mbedtls_pem_read_buffer() --- ChangeLog | 7 +++++++ library/pem.c | 20 +++++++++++--------- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/ChangeLog b/ChangeLog index d4cf85b7e..ea7ce072f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,12 @@ mbed TLS ChangeLog (Sorted per branch, date) += mbed TLS 1.3.x branch released xxxx-xx-xx + +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. + = mbed TLS 1.3.18 branch 2016-10-17 Security diff --git a/library/pem.c b/library/pem.c index 054fcffb8..1fe238726 100644 --- a/library/pem.c +++ b/library/pem.c @@ -250,7 +250,7 @@ int pem_read_buffer( pem_context *ctx, const char *header, const char *footer, enc = 0; - if( memcmp( s1, "Proc-Type: 4,ENCRYPTED", 22 ) == 0 ) + if( s2 - s1 >= 22 && memcmp( s1, "Proc-Type: 4,ENCRYPTED", 22 ) == 0 ) { #if defined(POLARSSL_MD5_C) && defined(POLARSSL_CIPHER_MODE_CBC) && \ ( defined(POLARSSL_DES_C) || defined(POLARSSL_AES_C) ) @@ -263,22 +263,22 @@ int pem_read_buffer( pem_context *ctx, const char *header, const char *footer, #if defined(POLARSSL_DES_C) - if( memcmp( s1, "DEK-Info: DES-EDE3-CBC,", 23 ) == 0 ) + if( s2 - s1 >= 23 && memcmp( s1, "DEK-Info: DES-EDE3-CBC,", 23 ) == 0 ) { enc_alg = POLARSSL_CIPHER_DES_EDE3_CBC; s1 += 23; - if( pem_get_iv( s1, pem_iv, 8 ) != 0 ) + if( s2 - s1 < 16 || pem_get_iv( s1, pem_iv, 8 ) != 0 ) return( POLARSSL_ERR_PEM_INVALID_ENC_IV ); s1 += 16; } - else if( memcmp( s1, "DEK-Info: DES-CBC,", 18 ) == 0 ) + else if( s2 - s1 >= 18 && memcmp( s1, "DEK-Info: DES-CBC,", 18 ) == 0 ) { enc_alg = POLARSSL_CIPHER_DES_CBC; s1 += 18; - if( pem_get_iv( s1, pem_iv, 8) != 0 ) + if( s2 - s1 < 16 || pem_get_iv( s1, pem_iv, 8) != 0 ) return( POLARSSL_ERR_PEM_INVALID_ENC_IV ); s1 += 16; @@ -286,9 +286,11 @@ int pem_read_buffer( pem_context *ctx, const char *header, const char *footer, #endif /* POLARSSL_DES_C */ #if defined(POLARSSL_AES_C) - if( memcmp( s1, "DEK-Info: AES-", 14 ) == 0 ) + if( s2 - s1 >= 14 && memcmp( s1, "DEK-Info: AES-", 14 ) == 0 ) { - if( memcmp( s1, "DEK-Info: AES-128-CBC,", 22 ) == 0 ) + if( s2 - s1 < 22 ) + return( POLARSSL_ERR_PEM_UNKNOWN_ENC_ALG ); + else if( memcmp( s1, "DEK-Info: AES-128-CBC,", 22 ) == 0 ) enc_alg = POLARSSL_CIPHER_AES_128_CBC; else if( memcmp( s1, "DEK-Info: AES-192-CBC,", 22 ) == 0 ) enc_alg = POLARSSL_CIPHER_AES_192_CBC; @@ -298,7 +300,7 @@ int pem_read_buffer( pem_context *ctx, const char *header, const char *footer, return( POLARSSL_ERR_PEM_UNKNOWN_ENC_ALG ); s1 += 22; - if( pem_get_iv( s1, pem_iv, 16 ) != 0 ) + if( s2 - s1 < 32 || pem_get_iv( s1, pem_iv, 16 ) != 0 ) return( POLARSSL_ERR_PEM_INVALID_ENC_IV ); s1 += 32; @@ -317,7 +319,7 @@ int pem_read_buffer( pem_context *ctx, const char *header, const char *footer, ( POLARSSL_AES_C || POLARSSL_DES_C ) */ } - if( s1 == s2 ) + if( s1 >= s2 ) return( POLARSSL_ERR_PEM_INVALID_DATA ); len = 0; From fada2e9f3e9f233b889bc711d0325b6be219e846 Mon Sep 17 00:00:00 2001 From: Andres AG Date: Mon, 24 Oct 2016 14:31:54 +0100 Subject: [PATCH 02/17] Add tests for overreads in pem_read_buffer() --- ChangeLog | 2 +- tests/suites/test_suite_pem.data | 9 +++++++++ tests/suites/test_suite_pem.function | 24 ++++++++++++++++++------ 3 files changed, 28 insertions(+), 7 deletions(-) diff --git a/ChangeLog b/ChangeLog index ea7ce072f..4cfcfeb43 100644 --- a/ChangeLog +++ b/ChangeLog @@ -4,7 +4,7 @@ mbed TLS ChangeLog (Sorted per branch, date) Bugfix * Fixed multiple buffer overreads in mbedtls_pem_read_buffer() when parsing - the input string in pem format to extract the different components. Found + the input string in PEM format to extract the different components. Found by Eyal Itkin. = mbed TLS 1.3.18 branch 2016-10-17 diff --git a/tests/suites/test_suite_pem.data b/tests/suites/test_suite_pem.data index 311ea9c15..9c7b30517 100644 --- a/tests/suites/test_suite_pem.data +++ b/tests/suites/test_suite_pem.data @@ -15,3 +15,12 @@ pem_write_buffer:"-----START TEST-----\n":"-----END TEST-----\n":"00010203040506 PEM write (exactly two lines + 1) pem_write_buffer:"-----START TEST-----\n":"-----END TEST-----\n":"000102030405060708090A0B0C0D0E0F000102030405060708090A0B0C0D0E0F000102030405060708090A0B0C0D0E0F000102030405060708090A0B0C0D0E0F000102030405060708090A0B0C0D0E0F000102030405060708090A0B0C0D0E0F00":"-----START TEST-----\nAAECAwQFBgcICQoLDA0ODwABAgMEBQYHCAkKCwwNDg8AAQIDBAUGBwgJCgsMDQ4P\nAAECAwQFBgcICQoLDA0ODwABAgMEBQYHCAkKCwwNDg8AAQIDBAUGBwgJCgsMDQ4P\nAA==\n-----END TEST-----\n" + +PEM read (DES-EDE3-CBC + invalid iv) +pem_read_buffer:"^":"$":"^\nProc-Type\: 4,ENCRYPTED\nDEK-Info\: DES-EDE3-CBC,00$":-4608 + +PEM read (DES-CBC + invalid iv) +pem_read_buffer:"^":"$":"^\nProc-Type\: 4,ENCRYPTED\nDEK-Info\: DES-CBC,00$":-4608 + +PEM read (unknown encryption algorithm) +pem_read_buffer:"^":"$":"^\nProc-Type\: 4,ENCRYPTED\nDEK-Info\: AES-,00$":-4736 diff --git a/tests/suites/test_suite_pem.function b/tests/suites/test_suite_pem.function index f8aab47c1..e0b767984 100644 --- a/tests/suites/test_suite_pem.function +++ b/tests/suites/test_suite_pem.function @@ -3,12 +3,7 @@ #include "polarssl/pem.h" /* END_HEADER */ -/* BEGIN_DEPENDENCIES - * depends_on:POLARSSL_PEM_WRITE_C - * END_DEPENDENCIES - */ - -/* BEGIN_CASE */ +/* BEGIN_CASE depends_on:POLARSSL_PEM_WRITE_C */ void pem_write_buffer( char *start, char *end, char *buf_str, char *result_str ) { unsigned char buf[5000]; @@ -38,3 +33,20 @@ exit: polarssl_free( check_buf ); } /* END_CASE */ + +/* BEGIN_CASE depends_on:POLARSSL_PEM_PARSE_C:POLARSSL_AES_C:POLARSSL_DES_C:POLARSSL_MD5_C:POLARSSL_CIPHER_MODE_CBC */ +void pem_read_buffer( char *header, char *footer, char *data, int ret ) +{ + pem_context ctx; + size_t use_len = 0; + + pem_init( &ctx ); + + TEST_ASSERT( pem_read_buffer( &ctx, header, footer, + (const unsigned char *)data, NULL, 0, + &use_len ) == ret ); + +exit: + pem_free( &ctx ); +} +/* END_CASE */ From 593e8b27937c266a38aa8f1eca1dd237d7b63a9d Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Wed, 18 Jan 2017 13:56:58 +0000 Subject: [PATCH 03/17] 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 +- 4 files changed, 15 insertions(+), 4 deletions(-) diff --git a/ChangeLog b/ChangeLog index d4cf85b7e..fb0d5fef0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,15 @@ mbed TLS ChangeLog (Sorted per branch, date) += mbed TLS 1.3.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 1.3.18 branch 2016-10-17 Security diff --git a/library/cipher.c b/library/cipher.c index b69d33106..7ea25cfc2 100644 --- a/library/cipher.c +++ b/library/cipher.c @@ -315,9 +315,9 @@ int cipher_update( cipher_context_t *ctx, const unsigned char *input, * If there is not enough data for a full block, cache it. */ if( ( ctx->operation == POLARSSL_DECRYPT && - ilen + ctx->unprocessed_len <= cipher_get_block_size( ctx ) ) || + ilen <= cipher_get_block_size( ctx ) - ctx->unprocessed_len ) || ( ctx->operation == POLARSSL_ENCRYPT && - ilen + ctx->unprocessed_len < cipher_get_block_size( ctx ) ) ) + ilen < cipher_get_block_size( ctx ) - 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 24adff08f..7b315e888 100644 --- a/library/ctr_drbg.c +++ b/library/ctr_drbg.c @@ -277,7 +277,8 @@ int ctr_drbg_reseed( ctr_drbg_context *ctx, unsigned char seed[CTR_DRBG_MAX_SEED_INPUT]; size_t seedlen = 0; - if( ctx->entropy_len + len > CTR_DRBG_MAX_SEED_INPUT ) + if( ctx->entropy_len > CTR_DRBG_MAX_SEED_INPUT || + len > CTR_DRBG_MAX_SEED_INPUT - ctx->entropy_len ) return( POLARSSL_ERR_CTR_DRBG_INPUT_TOO_BIG ); memset( seed, 0, CTR_DRBG_MAX_SEED_INPUT ); diff --git a/library/md2.c b/library/md2.c index 110cd95bc..2ac7eba61 100644 --- a/library/md2.c +++ b/library/md2.c @@ -155,7 +155,7 @@ void md2_update( md2_context *ctx, const unsigned char *input, size_t ilen ) while( ilen > 0 ) { - if( ctx->left + ilen > 16 ) + if( ilen > 16 - ctx->left ) fill = 16 - ctx->left; else fill = ilen; From 3e3698ca307b0991c573a007e0d773b48c83e862 Mon Sep 17 00:00:00 2001 From: Andres AG Date: Wed, 18 Jan 2017 17:21:03 +0000 Subject: [PATCH 04/17] Fix integer overflow in 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 d4cf85b7e..124d056fc 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,11 @@ mbed TLS ChangeLog (Sorted per branch, date) += mbed TLS 1.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 1.3.18 branch 2016-10-17 Security diff --git a/library/base64.c b/library/base64.c index 7de87e51c..3de67f090 100644 --- a/library/base64.c +++ b/library/base64.c @@ -198,7 +198,7 @@ int base64_decode( unsigned char *dst, size_t *dlen, return( 0 ); } - n = ( ( n * 6 ) + 7 ) >> 3; + n = ( 6 * ( n >> 3 ) ) + ( ( 6 * ( n & 0x7 ) + 7 ) >> 3 ); n -= j; if( dst == NULL || *dlen < n ) From 2d56a827ccf8d693db34f4cdb37202f34fb4048f Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Thu, 2 Feb 2017 08:46:53 +0000 Subject: [PATCH 05/17] 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 3de67f090..ba6926083 100644 --- a/library/base64.c +++ b/library/base64.c @@ -198,6 +198,10 @@ int base64_decode( unsigned char *dst, size_t *dlen, 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; From 6aa732f25a86719d60e56b1b7ff0d7168f298063 Mon Sep 17 00:00:00 2001 From: Andres AG Date: Thu, 2 Feb 2017 14:36:49 +0000 Subject: [PATCH 06/17] Fix generate_code.pl to handle escaped : --- tests/scripts/generate_code.pl | 2 +- tests/suites/test_suite_pem.data | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/scripts/generate_code.pl b/tests/scripts/generate_code.pl index 078e82df9..e13a2d0da 100755 --- a/tests/scripts/generate_code.pl +++ b/tests/scripts/generate_code.pl @@ -139,7 +139,7 @@ while($test_cases =~ /\/\* BEGIN_CASE *([\w:]*) \*\/\n(.*?)\n\/\* END_CASE \*\// $param_defs .= " char *param$i = params[$i];\n"; $param_checks .= " if( verify_string( ¶m$i ) != 0 ) return( 2 );\n"; push @dispatch_params, "param$i"; - $mapping_regex .= ":[^:\n]+"; + $mapping_regex .= ":(?:\\\\.|[^:\n])+"; } else { diff --git a/tests/suites/test_suite_pem.data b/tests/suites/test_suite_pem.data index 9c7b30517..b5f63e550 100644 --- a/tests/suites/test_suite_pem.data +++ b/tests/suites/test_suite_pem.data @@ -17,10 +17,10 @@ PEM write (exactly two lines + 1) pem_write_buffer:"-----START TEST-----\n":"-----END TEST-----\n":"000102030405060708090A0B0C0D0E0F000102030405060708090A0B0C0D0E0F000102030405060708090A0B0C0D0E0F000102030405060708090A0B0C0D0E0F000102030405060708090A0B0C0D0E0F000102030405060708090A0B0C0D0E0F00":"-----START TEST-----\nAAECAwQFBgcICQoLDA0ODwABAgMEBQYHCAkKCwwNDg8AAQIDBAUGBwgJCgsMDQ4P\nAAECAwQFBgcICQoLDA0ODwABAgMEBQYHCAkKCwwNDg8AAQIDBAUGBwgJCgsMDQ4P\nAA==\n-----END TEST-----\n" PEM read (DES-EDE3-CBC + invalid iv) -pem_read_buffer:"^":"$":"^\nProc-Type\: 4,ENCRYPTED\nDEK-Info\: DES-EDE3-CBC,00$":-4608 +pem_read_buffer:"^":"$":"^\nProc-Type\: 4,ENCRYPTED\nDEK-Info\: DES-EDE3-CBC,00$":POLARSSL_ERR_PEM_INVALID_ENC_IV PEM read (DES-CBC + invalid iv) -pem_read_buffer:"^":"$":"^\nProc-Type\: 4,ENCRYPTED\nDEK-Info\: DES-CBC,00$":-4608 +pem_read_buffer:"^":"$":"^\nProc-Type\: 4,ENCRYPTED\nDEK-Info\: DES-CBC,00$":POLARSSL_ERR_PEM_INVALID_ENC_IV PEM read (unknown encryption algorithm) -pem_read_buffer:"^":"$":"^\nProc-Type\: 4,ENCRYPTED\nDEK-Info\: AES-,00$":-4736 +pem_read_buffer:"^":"$":"^\nProc-Type\: 4,ENCRYPTED\nDEK-Info\: AES-,00$":POLARSSL_ERR_PEM_UNKNOWN_ENC_ALG From 50b4b12f9f423bcd0dead5c395c69c4ee8acdb8e Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Thu, 2 Feb 2017 15:01:24 +0000 Subject: [PATCH 07/17] Fix curves.pl script to build The script, `tests/scripts/curves.pl` was broken, and did not build due to the make command not having been updated with the change from polarssl to mbed TLS. --- tests/scripts/curves.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/scripts/curves.pl b/tests/scripts/curves.pl index 1f489a387..25e43d896 100755 --- a/tests/scripts/curves.pl +++ b/tests/scripts/curves.pl @@ -34,7 +34,7 @@ for my $curve (@curves) { system( "scripts/config.pl unset $curve" ) and abort "Failed to disable $curve\n"; - system( "make polarssl" ) and abort "Failed to build lib: $curve\n"; + system( "make lib" ) and abort "Failed to build lib: $curve\n"; system( "cd tests && make" ) and abort "Failed to build tests: $curve\n"; system( "make $test" ) and abort "Failed test suite: $curve\n"; From 5cf7f388066dffa0f8ee57ad214b070e6e075472 Mon Sep 17 00:00:00 2001 From: Andres AG Date: Fri, 3 Feb 2017 13:00:02 +0000 Subject: [PATCH 08/17] Add lib target to library/CMakeLists.txt --- library/CMakeLists.txt | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/library/CMakeLists.txt b/library/CMakeLists.txt index 8ccc7a391..d98fc716a 100644 --- a/library/CMakeLists.txt +++ b/library/CMakeLists.txt @@ -136,10 +136,18 @@ endif(USE_SHARED_MBEDTLS_LIBRARY) if(UNIX) add_custom_target(polarssl - DEPENDS mbedtls # TODO: and mbedtls_static is shared is defined + DEPENDS mbedtls COMMAND ${CMAKE_SOURCE_DIR}/scripts/polarssl_symlinks.sh ${CMAKE_BINARY_DIR}/library ) + add_custom_target(lib + DEPENDS polarssl + ) + + set_directory_properties(PROPERTIES + ADDITIONAL_MAKE_CLEAN_FILES "${CMAKE_BINARY_DIR}/library/libpolarssl.a" + ) + if(USE_STATIC_MBEDTLS_LIBRARY AND USE_SHARED_MBEDTLS_LIBRARY) add_dependencies(polarssl mbedtls_static) endif() From d9c8f26f8bfbbc9d704371639124a7f6c2914512 Mon Sep 17 00:00:00 2001 From: Simon B Date: Thu, 10 Nov 2016 13:19:42 +0000 Subject: [PATCH 09/17] Fix for MSVC Compiler warnings Fixes Microsoft Visual C compiler warnings in multiple files. All issues with type mismatches. --- library/ccm.c | 6 ++++-- library/ssl_srv.c | 10 ++++++++++ library/ssl_tls.c | 2 +- library/x509_crt.c | 2 +- 4 files changed, 16 insertions(+), 4 deletions(-) diff --git a/library/ccm.c b/library/ccm.c index e397e0a42..bc3700f09 100644 --- a/library/ccm.c +++ b/library/ccm.c @@ -140,7 +140,7 @@ static int ccm_auth_crypt( ccm_context *ctx, int mode, size_t length, { int ret; unsigned char i; - unsigned char q = 16 - 1 - iv_len; + unsigned char q; size_t len_left, olen; unsigned char b[16]; unsigned char y[16]; @@ -163,6 +163,8 @@ static int ccm_auth_crypt( ccm_context *ctx, int mode, size_t length, if( add_len > 0xFF00 ) return( POLARSSL_ERR_CCM_BAD_INPUT ); + q = 16 - 1 - (unsigned char) iv_len; + /* * First block B_0: * 0 .. 0 flags @@ -254,7 +256,7 @@ static int ccm_auth_crypt( ccm_context *ctx, int mode, size_t length, while( len_left > 0 ) { - unsigned char use_len = len_left > 16 ? 16 : len_left; + size_t use_len = len_left > 16 ? 16 : len_left; if( mode == CCM_ENCRYPT ) { diff --git a/library/ssl_srv.c b/library/ssl_srv.c index f0a88fe2d..90d5ac7ff 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -2981,7 +2981,17 @@ static int ssl_parse_encrypted_pms( ssl_context *ssl, ssl->handshake->pmslen = 48; /* mask = diff ? 0xff : 0x00 */ + /* MSVC has a warning about unary minus on unsigned, but this is + * well-defined and precisely what we want to do here */ +#if defined(_MSC_VER) +#pragma warning( push ) +#pragma warning( disable : 4146 ) +#endif mask = - ( diff | - diff ) >> ( sizeof( unsigned int ) * 8 - 1 ); +#if defined(_MSC_VER) +#pragma warning( pop ) +#endif + for( i = 0; i < ssl->handshake->pmslen; i++ ) pms[i] = ( mask & fake_pms[i] ) | ( (~mask) & peer_pms[i] ); diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 0dd4a6c56..860499799 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1484,7 +1484,7 @@ static int ssl_decrypt_buf( ssl_context *ssl ) unsigned char add_data[13]; unsigned char taglen = ssl->transform_in->ciphersuite_info->flags & POLARSSL_CIPHERSUITE_SHORT_TAG ? 8 : 16; - unsigned char explicit_iv_len = ssl->transform_in->ivlen - + size_t explicit_iv_len = ssl->transform_in->ivlen - ssl->transform_in->fixed_ivlen; if( ssl->in_msglen < (size_t) explicit_iv_len + taglen ) diff --git a/library/x509_crt.c b/library/x509_crt.c index b7c73df1d..4b831aed3 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -988,7 +988,7 @@ int x509_crt_parse_path( x509_crt *chain, const char *path ) p = filename + len; filename[len++] = '*'; - w_ret = MultiByteToWideChar( CP_ACP, 0, filename, len, szDir, + w_ret = MultiByteToWideChar( CP_ACP, 0, filename, (int)len, szDir, MAX_PATH - 3 ); if( w_ret == 0 ) return( POLARSSL_ERR_X509_BAD_INPUT_DATA ); From 29b43737ba22ce86934cc5f63107b14cd0eb8f79 Mon Sep 17 00:00:00 2001 From: Andres AG Date: Wed, 7 Dec 2016 16:08:04 +0000 Subject: [PATCH 10/17] Fix unused variable/function compilation warnings This PR fixes a number of unused variable/function compilation warnings that arise when using a config.h that does not define the macro POLARSSL_PEM_PARSE_C. --- ChangeLog | 3 +++ library/pem.c | 2 +- library/x509_csr.c | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index 4cfcfeb43..2e25e1ca9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -6,6 +6,9 @@ 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. + * Fix unused variable/function compilation warnings in pem.c and x509_csr.c + that are reported when building mbed TLS with a config.h that does not + define POLARSSL_PEM_PARSE_C. #562 = mbed TLS 1.3.18 branch 2016-10-17 diff --git a/library/pem.c b/library/pem.c index 1fe238726..b2c16c292 100644 --- a/library/pem.c +++ b/library/pem.c @@ -45,12 +45,12 @@ #define polarssl_free free #endif +#if defined(POLARSSL_PEM_PARSE_C) /* Implementation that should never be optimized out by the compiler */ static void polarssl_zeroize( void *v, size_t n ) { volatile unsigned char *p = v; while( n-- ) *p++ = 0; } -#if defined(POLARSSL_PEM_PARSE_C) void pem_init( pem_context *ctx ) { memset( ctx, 0, sizeof( pem_context ) ); diff --git a/library/x509_csr.c b/library/x509_csr.c index 558b078b7..9bdfe884f 100644 --- a/library/x509_csr.c +++ b/library/x509_csr.c @@ -260,8 +260,8 @@ int x509_csr_parse_der( x509_csr *csr, */ int x509_csr_parse( x509_csr *csr, const unsigned char *buf, size_t buflen ) { - int ret; #if defined(POLARSSL_PEM_PARSE_C) + int ret; size_t use_len; pem_context pem; #endif From cfad1812508dd8e1baf9c99514c89bda5ab1cd10 Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Wed, 18 Jan 2017 13:56:58 +0000 Subject: [PATCH 11/17] 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 +- 4 files changed, 11 insertions(+), 4 deletions(-) diff --git a/ChangeLog b/ChangeLog index 2e25e1ca9..545893fb6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -9,6 +9,12 @@ Bugfix * Fix unused variable/function compilation warnings in pem.c and x509_csr.c that are reported when building mbed TLS with a config.h that does not define POLARSSL_PEM_PARSE_C. #562 + * 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 1.3.18 branch 2016-10-17 diff --git a/library/cipher.c b/library/cipher.c index b69d33106..7ea25cfc2 100644 --- a/library/cipher.c +++ b/library/cipher.c @@ -315,9 +315,9 @@ int cipher_update( cipher_context_t *ctx, const unsigned char *input, * If there is not enough data for a full block, cache it. */ if( ( ctx->operation == POLARSSL_DECRYPT && - ilen + ctx->unprocessed_len <= cipher_get_block_size( ctx ) ) || + ilen <= cipher_get_block_size( ctx ) - ctx->unprocessed_len ) || ( ctx->operation == POLARSSL_ENCRYPT && - ilen + ctx->unprocessed_len < cipher_get_block_size( ctx ) ) ) + ilen < cipher_get_block_size( ctx ) - 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 24adff08f..7b315e888 100644 --- a/library/ctr_drbg.c +++ b/library/ctr_drbg.c @@ -277,7 +277,8 @@ int ctr_drbg_reseed( ctr_drbg_context *ctx, unsigned char seed[CTR_DRBG_MAX_SEED_INPUT]; size_t seedlen = 0; - if( ctx->entropy_len + len > CTR_DRBG_MAX_SEED_INPUT ) + if( ctx->entropy_len > CTR_DRBG_MAX_SEED_INPUT || + len > CTR_DRBG_MAX_SEED_INPUT - ctx->entropy_len ) return( POLARSSL_ERR_CTR_DRBG_INPUT_TOO_BIG ); memset( seed, 0, CTR_DRBG_MAX_SEED_INPUT ); diff --git a/library/md2.c b/library/md2.c index 110cd95bc..2ac7eba61 100644 --- a/library/md2.c +++ b/library/md2.c @@ -155,7 +155,7 @@ void md2_update( md2_context *ctx, const unsigned char *input, size_t ilen ) while( ilen > 0 ) { - if( ctx->left + ilen > 16 ) + if( ilen > 16 - ctx->left ) fill = 16 - ctx->left; else fill = ilen; From 7ded99ff6475d04d02344bfab9a00e482526dbe2 Mon Sep 17 00:00:00 2001 From: Andres AG Date: Wed, 18 Jan 2017 17:21:03 +0000 Subject: [PATCH 12/17] Fix integer overflow in 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 | 2 ++ library/base64.c | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 545893fb6..81af3700a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -15,6 +15,8 @@ Bugfix 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. + * Fixed potential arithmetic overflow in mbedtls_base64_decode() that could + cause buffer bound checks to be bypassed. Found by Eyal Itkin. = mbed TLS 1.3.18 branch 2016-10-17 diff --git a/library/base64.c b/library/base64.c index 7de87e51c..3de67f090 100644 --- a/library/base64.c +++ b/library/base64.c @@ -198,7 +198,7 @@ int base64_decode( unsigned char *dst, size_t *dlen, return( 0 ); } - n = ( ( n * 6 ) + 7 ) >> 3; + n = ( 6 * ( n >> 3 ) ) + ( ( 6 * ( n & 0x7 ) + 7 ) >> 3 ); n -= j; if( dst == NULL || *dlen < n ) From b2bad3c79be5712a2213fc13d092898c7889988f Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Wed, 1 Feb 2017 12:38:44 +0000 Subject: [PATCH 13/17] Adds dl link library to OpenSSL example builds The example o_p_test uses OpenSSL. On some platforms that fails to build unless the dl library is included as a static link library. --- programs/test/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/test/CMakeLists.txt b/programs/test/CMakeLists.txt index da3376e64..500043146 100644 --- a/programs/test/CMakeLists.txt +++ b/programs/test/CMakeLists.txt @@ -31,7 +31,7 @@ install(TARGETS selftest benchmark ssl_test ssl_cert_test if(OPENSSL_FOUND) add_executable(o_p_test o_p_test.c) include_directories(${OPENSSL_INCLUDE_DIR}) - target_link_libraries(o_p_test ${libs} ${OPENSSL_LIBRARIES}) + target_link_libraries(o_p_test ${libs} ${OPENSSL_LIBRARIES} ${CMAKE_DL_LIBS}) install(TARGETS o_p_test DESTINATION "bin" From ba32ebf7f4ad336cb1f5429fba884d70bf94e39d Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Thu, 2 Feb 2017 08:46:53 +0000 Subject: [PATCH 14/17] 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 3de67f090..ba6926083 100644 --- a/library/base64.c +++ b/library/base64.c @@ -198,6 +198,10 @@ int base64_decode( unsigned char *dst, size_t *dlen, 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; From e6254531d06763777e2f0ff9ea072847459b42e7 Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Thu, 2 Feb 2017 15:01:24 +0000 Subject: [PATCH 15/17] Fix curves.pl script to build The script, `tests/scripts/curves.pl` was broken, and did not build due to the make command not having been updated with the change from polarssl to mbed TLS. --- tests/scripts/curves.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/scripts/curves.pl b/tests/scripts/curves.pl index 1f489a387..25e43d896 100755 --- a/tests/scripts/curves.pl +++ b/tests/scripts/curves.pl @@ -34,7 +34,7 @@ for my $curve (@curves) { system( "scripts/config.pl unset $curve" ) and abort "Failed to disable $curve\n"; - system( "make polarssl" ) and abort "Failed to build lib: $curve\n"; + system( "make lib" ) and abort "Failed to build lib: $curve\n"; system( "cd tests && make" ) and abort "Failed to build tests: $curve\n"; system( "make $test" ) and abort "Failed test suite: $curve\n"; From 851dcc96d4a19b66668abfec1b0a35b6a68c88cd Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Thu, 2 Feb 2017 16:53:50 +0000 Subject: [PATCH 16/17] Add credit to Changelog for #562 --- ChangeLog | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 81af3700a..3c7a423b2 100644 --- a/ChangeLog +++ b/ChangeLog @@ -8,7 +8,7 @@ Bugfix by Eyal Itkin. * Fix unused variable/function compilation warnings in pem.c and x509_csr.c that are reported when building mbed TLS with a config.h that does not - define POLARSSL_PEM_PARSE_C. #562 + define POLARSSL_PEM_PARSE_C. Found by omnium21. #562 * 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 From 63c4fda9cfd52606902565c69692f4d7bdd176db Mon Sep 17 00:00:00 2001 From: Andres AG Date: Fri, 3 Feb 2017 13:00:02 +0000 Subject: [PATCH 17/17] Add lib target to library/CMakeLists.txt --- library/CMakeLists.txt | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/library/CMakeLists.txt b/library/CMakeLists.txt index 8ccc7a391..d98fc716a 100644 --- a/library/CMakeLists.txt +++ b/library/CMakeLists.txt @@ -136,10 +136,18 @@ endif(USE_SHARED_MBEDTLS_LIBRARY) if(UNIX) add_custom_target(polarssl - DEPENDS mbedtls # TODO: and mbedtls_static is shared is defined + DEPENDS mbedtls COMMAND ${CMAKE_SOURCE_DIR}/scripts/polarssl_symlinks.sh ${CMAKE_BINARY_DIR}/library ) + add_custom_target(lib + DEPENDS polarssl + ) + + set_directory_properties(PROPERTIES + ADDITIONAL_MAKE_CLEAN_FILES "${CMAKE_BINARY_DIR}/library/libpolarssl.a" + ) + if(USE_STATIC_MBEDTLS_LIBRARY AND USE_SHARED_MBEDTLS_LIBRARY) add_dependencies(polarssl mbedtls_static) endif()