From a697bf503ad97c74b43d2ff01aa07c6629d23c16 Mon Sep 17 00:00:00 2001 From: Simon B Date: Thu, 10 Nov 2016 13:19:42 +0000 Subject: [PATCH 01/10] 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 f0a401f0805d93f85939448e6f7750aed28f6d85 Mon Sep 17 00:00:00 2001 From: Andres AG Date: Wed, 7 Dec 2016 16:08:04 +0000 Subject: [PATCH 02/10] 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 | 7 +++++++ library/pem.c | 2 +- library/x509_csr.c | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index d4cf85b7e..08f705eff 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,12 @@ mbed TLS ChangeLog (Sorted per branch, date) += mbed TLS x.x.x branch xxxx-xx-xx + +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 + = mbed TLS 1.3.18 branch 2016-10-17 Security diff --git a/library/pem.c b/library/pem.c index 054fcffb8..ac8311691 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 593e8b27937c266a38aa8f1eca1dd237d7b63a9d Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Wed, 18 Jan 2017 13:56:58 +0000 Subject: [PATCH 03/10] 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/10] 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 40d8cc7181578b54c5aa44a0c5e0b990314e73c3 Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Wed, 1 Feb 2017 12:38:44 +0000 Subject: [PATCH 05/10] 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 2d56a827ccf8d693db34f4cdb37202f34fb4048f Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Thu, 2 Feb 2017 08:46:53 +0000 Subject: [PATCH 06/10] 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 50b4b12f9f423bcd0dead5c395c69c4ee8acdb8e Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Thu, 2 Feb 2017 15:01:24 +0000 Subject: [PATCH 07/10] 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 1842a006880832ff2e9447fa98ff6f23cc16d84f Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Thu, 2 Feb 2017 15:01:24 +0000 Subject: [PATCH 08/10] 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 df33a6a8056be7fcb8a1784fa026d0308f19dc7b Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Thu, 2 Feb 2017 16:53:50 +0000 Subject: [PATCH 09/10] Add credit to Changelog for #562 --- ChangeLog | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 08f705eff..1e1420ab0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -5,7 +5,7 @@ mbed TLS ChangeLog (Sorted per branch, date) 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 + define POLARSSL_PEM_PARSE_C. Found by omnium21. #562 = mbed TLS 1.3.18 branch 2016-10-17 From 5cf7f388066dffa0f8ee57ad214b070e6e075472 Mon Sep 17 00:00:00 2001 From: Andres AG Date: Fri, 3 Feb 2017 13:00:02 +0000 Subject: [PATCH 10/10] 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()