From f267830d6fd27a6dccdae502a2be330e3d2a5fe1 Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Fri, 4 Nov 2016 12:23:11 +0000 Subject: [PATCH 1/8] Fix multiple erroneously named source files in comments This fixes many incorrect references to filenames in the comments in config.h. --- include/mbedtls/config.h | 62 ++++++++++++++++++++-------------------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index e77cf2623..c720cb98e 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -1443,7 +1443,7 @@ * library/pkwrite.c * library/x509_create.c * library/x509write_crt.c - * library/mbedtls_x509write_csr.c + * library/x509write_csr.c */ #define MBEDTLS_ASN1_WRITE_C @@ -1771,7 +1771,7 @@ * * Enable the generic message digest layer. * - * Module: library/mbedtls_md.c + * Module: library/md.c * Caller: * * Uncomment to enable generic message digest wrappers. @@ -1783,7 +1783,7 @@ * * Enable the MD2 hash algorithm. * - * Module: library/mbedtls_md2.c + * Module: library/md2.c * Caller: * * Uncomment to enable support for (rare) MD2-signed X.509 certs. @@ -1795,7 +1795,7 @@ * * Enable the MD4 hash algorithm. * - * Module: library/mbedtls_md4.c + * Module: library/md4.c * Caller: * * Uncomment to enable support for (rare) MD4-signed X.509 certs. @@ -1807,8 +1807,8 @@ * * Enable the MD5 hash algorithm. * - * Module: library/mbedtls_md5.c - * Caller: library/mbedtls_md.c + * Module: library/md5.c + * Caller: library/md.c * library/pem.c * library/ssl_tls.c * @@ -1857,11 +1857,11 @@ * library/rsa.c * library/x509.c * library/x509_create.c - * library/mbedtls_x509_crl.c - * library/mbedtls_x509_crt.c - * library/mbedtls_x509_csr.c + * library/x509_crl.c + * library/x509_crt.c + * library/x509_csr.c * library/x509write_crt.c - * library/mbedtls_x509write_csr.c + * library/x509write_csr.c * * This modules translates between OIDs and internal values. */ @@ -1889,9 +1889,9 @@ * Module: library/pem.c * Caller: library/dhm.c * library/pkparse.c - * library/mbedtls_x509_crl.c - * library/mbedtls_x509_crt.c - * library/mbedtls_x509_csr.c + * library/x509_crl.c + * library/x509_crt.c + * library/x509_csr.c * * Requires: MBEDTLS_BASE64_C * @@ -1907,7 +1907,7 @@ * Module: library/pem.c * Caller: library/pkwrite.c * library/x509write_crt.c - * library/mbedtls_x509write_csr.c + * library/x509write_csr.c * * Requires: MBEDTLS_BASE64_C * @@ -1937,8 +1937,8 @@ * Enable the generic public (asymetric) key parser. * * Module: library/pkparse.c - * Caller: library/mbedtls_x509_crt.c - * library/mbedtls_x509_csr.c + * Caller: library/x509_crt.c + * library/x509_csr.c * * Requires: MBEDTLS_PK_C * @@ -2029,8 +2029,8 @@ * * Enable the RIPEMD-160 hash algorithm. * - * Module: library/mbedtls_ripemd160.c - * Caller: library/mbedtls_md.c + * Module: library/ripemd160.c + * Caller: library/md.c * */ #define MBEDTLS_RIPEMD160_C @@ -2058,8 +2058,8 @@ * * Enable the SHA1 cryptographic hash algorithm. * - * Module: library/mbedtls_sha1.c - * Caller: library/mbedtls_md.c + * Module: library/sha1.c + * Caller: library/md.c * library/ssl_cli.c * library/ssl_srv.c * library/ssl_tls.c @@ -2074,9 +2074,9 @@ * * Enable the SHA-224 and SHA-256 cryptographic hash algorithms. * - * Module: library/mbedtls_sha256.c + * Module: library/sha256.c * Caller: library/entropy.c - * library/mbedtls_md.c + * library/md.c * library/ssl_cli.c * library/ssl_srv.c * library/ssl_tls.c @@ -2091,9 +2091,9 @@ * * Enable the SHA-384 and SHA-512 cryptographic hash algorithms. * - * Module: library/mbedtls_sha512.c + * Module: library/sha512.c * Caller: library/entropy.c - * library/mbedtls_md.c + * library/md.c * library/ssl_cli.c * library/ssl_srv.c * @@ -2229,9 +2229,9 @@ * Enable X.509 core for using certificates. * * Module: library/x509.c - * Caller: library/mbedtls_x509_crl.c - * library/mbedtls_x509_crt.c - * library/mbedtls_x509_csr.c + * Caller: library/x509_crl.c + * library/x509_crt.c + * library/x509_csr.c * * Requires: MBEDTLS_ASN1_PARSE_C, MBEDTLS_BIGNUM_C, MBEDTLS_OID_C, * MBEDTLS_PK_PARSE_C @@ -2245,7 +2245,7 @@ * * Enable X.509 certificate parsing. * - * Module: library/mbedtls_x509_crt.c + * Module: library/x509_crt.c * Caller: library/ssl_cli.c * library/ssl_srv.c * library/ssl_tls.c @@ -2261,8 +2261,8 @@ * * Enable X.509 CRL parsing. * - * Module: library/mbedtls_x509_crl.c - * Caller: library/mbedtls_x509_crt.c + * Module: library/x509_crl.c + * Caller: library/x509_crt.c * * Requires: MBEDTLS_X509_USE_C * @@ -2275,7 +2275,7 @@ * * Enable X.509 Certificate Signing Request (CSR) parsing. * - * Module: library/mbedtls_x509_csr.c + * Module: library/x509_csr.c * Caller: library/x509_crt_write.c * * Requires: MBEDTLS_X509_USE_C From 44ea01110cf3f9e300c8f0d0f0eeab1cd4d379be Mon Sep 17 00:00:00 2001 From: Nicholas Wilson Date: Sat, 14 Nov 2015 13:09:01 +0000 Subject: [PATCH 2/8] Allow test suites to be run on Windows For a start, they don't even compile with Visual Studio due to strcasecmp being missing. Secondly, on Windows Perl scripts aren't executable and have to be run using the Perl interpreter directly; thankfully CMake is able to find cygwin Perl straight away without problems. --- tests/CMakeLists.txt | 7 ++++++- tests/suites/helpers.function | 2 ++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 47fedd5f7..b1b1ea426 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -10,6 +10,11 @@ if(ENABLE_ZLIB_SUPPORT) set(libs ${libs} ${ZLIB_LIBRARIES}) endif(ENABLE_ZLIB_SUPPORT) +find_package(Perl) +if(NOT PERL_FOUND) + message(FATAL_ERROR "Cannot build test suites without Perl") +endif() + function(add_test_suite suite_name) if(ARGV1) set(data_name ${ARGV1}) @@ -19,7 +24,7 @@ function(add_test_suite suite_name) add_custom_command( OUTPUT test_suite_${data_name}.c - COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/scripts/generate_code.pl ${CMAKE_CURRENT_SOURCE_DIR}/suites test_suite_${suite_name} test_suite_${data_name} + COMMAND ${PERL_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/scripts/generate_code.pl ${CMAKE_CURRENT_SOURCE_DIR}/suites test_suite_${suite_name} test_suite_${data_name} DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/scripts/generate_code.pl mbedtls suites/helpers.function suites/main_test.function suites/test_suite_${suite_name}.function suites/test_suite_${data_name}.data ) diff --git a/tests/suites/helpers.function b/tests/suites/helpers.function index 8f681dbd4..6af918cad 100644 --- a/tests/suites/helpers.function +++ b/tests/suites/helpers.function @@ -15,6 +15,8 @@ #ifdef _MSC_VER #include typedef UINT32 uint32_t; +#define strncasecmp _strnicmp +#define strcasecmp _stricmp #else #include #endif From 635f215145f9ffc3315068143f5c387a2f6637c0 Mon Sep 17 00:00:00 2001 From: Simon B Date: Thu, 3 Nov 2016 01:11:37 +0000 Subject: [PATCH 3/8] Fix compiler warning with MSVC Fixes compiler warnings found with Microsoft Visual Studio 2015 (and earlier versions). --- library/x509_crt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/x509_crt.c b/library/x509_crt.c index 9251aed31..81402ba55 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -1121,7 +1121,7 @@ int mbedtls_x509_crt_parse_path( mbedtls_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( MBEDTLS_ERR_X509_BAD_INPUT_DATA ); From e2e2db434893bbea70f6a02c9d76b12845f17fda Mon Sep 17 00:00:00 2001 From: Simon B Date: Thu, 3 Nov 2016 01:12:50 +0000 Subject: [PATCH 4/8] Fix config of compiler warning flags with MSVC Compiler warnings were being configured twice and not suppressed on the test suites with Microsoft Visual Studio. --- CMakeLists.txt | 4 +++- tests/CMakeLists.txt | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 499ccff90..6b3182bfb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -54,7 +54,9 @@ if(CMAKE_COMPILER_IS_CLANG) endif(CMAKE_COMPILER_IS_CLANG) if(MSVC) - set(CMAKE_C_FLAGS_CHECK "/WX") + # Strictest warnings, and treat as errors + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} /W3") + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} /WX") endif(MSVC) if(CMAKE_BUILD_TYPE STREQUAL "Coverage") diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index b1b1ea426..8608da14d 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -39,7 +39,9 @@ if(CMAKE_COMPILER_IS_GNUCC OR CMAKE_COMPILER_IS_CLANG) endif(CMAKE_COMPILER_IS_GNUCC OR CMAKE_COMPILER_IS_CLANG) if(MSVC) - set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} /w") # no warnings here + # If a warning level has been defined, suppress all warnings for test code + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} /W0") + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} /WX-") endif(MSVC) add_test_suite(aes aes.ecb) From 41ce6e60766a4a92241efa2571f1b5219dc02e9c Mon Sep 17 00:00:00 2001 From: Simon B Date: Sat, 12 Nov 2016 22:34:10 +0000 Subject: [PATCH 5/8] Remove need for elevated command line in Windows Changes use of mklink in Windows test builds, to create junctions instead of directory symbolic links. This removes the need for an elevated command prompt when running cmake to create the Visual Studio project files. --- tests/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 8608da14d..96ce7f94f 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -112,7 +112,7 @@ if (NOT ${CMAKE_CURRENT_BINARY_DIR} STREQUAL ${CMAKE_CURRENT_SOURCE_DIR}) if (CMAKE_HOST_UNIX) set(command ln -s ${target} ${link}) else() - set(command cmd.exe /c mklink /d ${link} ${target}) + set(command cmd.exe /c mklink /j ${link} ${target}) endif() execute_process(COMMAND ${command} From ba2fda645affceb7fafde8aa0624ccf12a5e59e7 Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Tue, 17 Jan 2017 23:24:02 +0000 Subject: [PATCH 6/8] 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 92d7977e0..0776a56d8 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.1.6 branch released 2016-10-17 Security diff --git a/library/cipher.c b/library/cipher.c index ccc068503..1523b07da 100644 --- a/library/cipher.c +++ b/library/cipher.c @@ -300,9 +300,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 <= mbedtls_cipher_get_block_size( ctx ) ) || + ilen <= mbedtls_cipher_get_block_size( ctx ) - ctx->unprocessed_len ) || ( ctx->operation == MBEDTLS_ENCRYPT && - ilen + ctx->unprocessed_len < mbedtls_cipher_get_block_size( ctx ) ) ) + ilen < mbedtls_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 aefddfa1d..646196eb1 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; From 81d126f92397511f1bd4bb6f88f43d2f892838cf Mon Sep 17 00:00:00 2001 From: Andres AG Date: Wed, 18 Jan 2017 17:21:03 +0000 Subject: [PATCH 7/8] 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 92d7977e0..13827accd 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.1.6 branch released 2016-10-17 Security diff --git a/library/base64.c b/library/base64.c index 3432e5fcd..d4fdf6e1a 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 df06c200b831c29337a28302049270ecce282e7e Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Thu, 2 Feb 2017 08:46:53 +0000 Subject: [PATCH 8/8] 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 d4fdf6e1a..4ed6b312a 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;