Merge fix IOTSSL-475 Potential buffer overflow
Two possible integer overflows (during << 2 or addition in BITS_TO_LIMB())
could result in far too few memory to be allocated, then overflowing the
buffer in the subsequent for loop.
Both integer overflows happen when slen is close to or greater than
SIZE_T_MAX >> 2 (ie 2^30 on a 32 bit system).
Note: one could also avoid those overflows by changing BITS_TO_LIMB(s << 2) to
CHARS_TO_LIMB(s >> 1) but the solution implemented looks more robust with
respect to future code changes.
diff --git a/ChangeLog b/ChangeLog
index 7560803..63cedaa 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,14 +1,28 @@
mbed TLS ChangeLog (Sorted per branch, date)
-= mbed TLS 2.1.2 released 2015-??-??
+= mbed TLS 2.1.2 released 2015-10-xx
Security
+ * Added fix for CVE-2015-xxxxx to prevent heap corruption due to buffer
+ overflow of the hostname or session ticket. Found by Guido Vranken.
+ * Fix potential double-free if mbedtls_ssl_set_hs_psk() is called more than
+ once in the same handhake and mbedtls_ssl_conf_psk() was used.
+ Found and patch provided by Guido Vranken. Cannot be forced remotely.
+ * Fix stack buffer overflow in pkcs12 decryption (used by
+ mbedtls_pk_parse_key(file)() when the password is > 129 bytes.
+ Found by Guido Vranken. Not triggerable remotely.
* Fix potential buffer overflow in mbedtls_mpi_read_string().
Found by Guido Vranken. Not exploitable remotely in the context of TLS,
but might be in other uses. On 32 bit machines, requires reading a string
of close to or larger than 1GB to exploit; on 64 bit machines, would require
reading a string of close to or larger than 2^62 bytes.
+Changes
+ * Added checking of hostname length in mbedtls_ssl_set_hostname() to ensure
+ domain names are compliant with RFC 1035.
+ * Fixed paths for check_config.h in example config files. (Found by bachp)
+ (#291)
+
= mbed TLS 2.1.1 released 2015-09-17
Security
@@ -17,7 +31,8 @@
https://securityblog.redhat.com/2015/09/02/factoring-rsa-keys-with-tls-perfect-forward-secrecy/
* Fix possible client-side NULL pointer dereference (read) when the client
tries to continue the handshake after it failed (a misuse of the API).
- (Found and patch provided by Fabian Foerg, Gotham Digital Science using afl-fuzz.)
+ (Found and patch provided by Fabian Foerg, Gotham Digital Science using
+ afl-fuzz.)
Bugfix
* Fix warning when using a 64bit platform. (found by embedthis) (#275)
@@ -122,7 +137,7 @@
mbedtls_gcm_init() -> mbedtls_gcm_setkey()
mbedtls_hmac_drbg_init() -> mbedtls_hmac_drbg_seed(_buf)()
mbedtls_ctr_drbg_init() -> mbedtls_ctr_drbg_seed()
- Note that for mbetls_ssl_setup(), you need to be done setting up the
+ Note that for mbedtls_ssl_setup(), you need to be done setting up the
ssl_config structure before calling it.
* Most ssl_set_xxx() functions (all except ssl_set_bio(), ssl_set_hostname(),
ssl_set_session() and ssl_set_client_transport_id(), plus
diff --git a/configs/config-ccm-psk-tls1_2.h b/configs/config-ccm-psk-tls1_2.h
index bd37091..aee10b8 100644
--- a/configs/config-ccm-psk-tls1_2.h
+++ b/configs/config-ccm-psk-tls1_2.h
@@ -80,6 +80,6 @@
*/
#define MBEDTLS_SSL_MAX_CONTENT_LEN 512
-#include "check_config.h"
+#include "mbedtls/check_config.h"
#endif /* MBEDTLS_CONFIG_H */
diff --git a/configs/config-picocoin.h b/configs/config-picocoin.h
index 4269872..26b24a9 100644
--- a/configs/config-picocoin.h
+++ b/configs/config-picocoin.h
@@ -66,6 +66,6 @@
#define MBEDTLS_SHA1_C
#define MBEDTLS_SHA256_C
-#include "check_config.h"
+#include "mbedtls/check_config.h"
#endif /* MBEDTLS_CONFIG_H */
diff --git a/include/mbedtls/ctr_drbg.h b/include/mbedtls/ctr_drbg.h
index a049330..059d3c5 100644
--- a/include/mbedtls/ctr_drbg.h
+++ b/include/mbedtls/ctr_drbg.h
@@ -111,7 +111,7 @@
/**
* \brief CTR_DRBG context initialization
- * Makes the context ready for mbetls_ctr_drbg_seed() or
+ * Makes the context ready for mbedtls_ctr_drbg_seed() or
* mbedtls_ctr_drbg_free().
*
* \param ctx CTR_DRBG context to be initialized
diff --git a/include/mbedtls/hmac_drbg.h b/include/mbedtls/hmac_drbg.h
index 4ffc646..e010558 100644
--- a/include/mbedtls/hmac_drbg.h
+++ b/include/mbedtls/hmac_drbg.h
@@ -98,7 +98,7 @@
/**
* \brief HMAC_DRBG context initialization
- * Makes the context ready for mbetls_hmac_drbg_seed(),
+ * Makes the context ready for mbedtls_hmac_drbg_seed(),
* mbedtls_hmac_drbg_seed_buf() or
* mbedtls_hmac_drbg_free().
*
diff --git a/include/mbedtls/pkcs11.h b/include/mbedtls/pkcs11.h
index aa549fd..7632929 100644
--- a/include/mbedtls/pkcs11.h
+++ b/include/mbedtls/pkcs11.h
@@ -54,7 +54,7 @@
} mbedtls_pkcs11_context;
/**
- * Initialize a mbetls_pkcs11_context.
+ * Initialize a mbedtls_pkcs11_context.
* (Just making memory references valid.)
*/
void mbedtls_pkcs11_init( mbedtls_pkcs11_context *ctx );
diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h
index 2d7beb3..a017ec0 100644
--- a/include/mbedtls/ssl.h
+++ b/include/mbedtls/ssl.h
@@ -139,6 +139,8 @@
#define MBEDTLS_SSL_TRANSPORT_STREAM 0 /*!< TLS */
#define MBEDTLS_SSL_TRANSPORT_DATAGRAM 1 /*!< DTLS */
+#define MBEDTLS_SSL_MAX_HOST_NAME_LEN 255 /*!< Maximum host name defined in RFC 1035 */
+
/* RFC 6066 section 4, see also mfl_code_to_length in ssl_tls.c
* NONE must be zero so that memset()ing structure to zero works */
#define MBEDTLS_SSL_MAX_FRAG_LEN_NONE 0 /*!< don't use this extension */
@@ -840,7 +842,7 @@
/**
* \brief Initialize an SSL context
- * Just makes the context ready for mbetls_ssl_setup() or
+ * Just makes the context ready for mbedtls_ssl_setup() or
* mbedtls_ssl_free()
*
* \param ssl SSL context
diff --git a/include/mbedtls/timing.h b/include/mbedtls/timing.h
index cc8754c..ae7a713 100644
--- a/include/mbedtls/timing.h
+++ b/include/mbedtls/timing.h
@@ -92,7 +92,7 @@
* (See \c mbedtls_timing_get_delay().)
*
* \param data Pointer to timing data
- * Must point to a valid \c mbetls_timing_delay_context struct.
+ * Must point to a valid \c mbedtls_timing_delay_context struct.
* \param int_ms First (intermediate) delay in milliseconds.
* \param fin_ms Second (final) delay in milliseconds.
* Pass 0 to cancel the current delay.
@@ -104,7 +104,7 @@
* (Memory helper: number of delays passed.)
*
* \param data Pointer to timing data
- * Must point to a valid \c mbetls_timing_delay_context struct.
+ * Must point to a valid \c mbedtls_timing_delay_context struct.
*
* \return -1 if cancelled (fin_ms = 0)
* 0 if none of the delays are passed,
diff --git a/library/pkcs12.c b/library/pkcs12.c
index f1777eb..7023b9d 100644
--- a/library/pkcs12.c
+++ b/library/pkcs12.c
@@ -86,6 +86,8 @@
return( 0 );
}
+#define PKCS12_MAX_PWDLEN 128
+
static int pkcs12_pbe_derive_key_iv( mbedtls_asn1_buf *pbe_params, mbedtls_md_type_t md_type,
const unsigned char *pwd, size_t pwdlen,
unsigned char *key, size_t keylen,
@@ -94,7 +96,10 @@
int ret, iterations;
mbedtls_asn1_buf salt;
size_t i;
- unsigned char unipwd[258];
+ unsigned char unipwd[PKCS12_MAX_PWDLEN * 2 + 2];
+
+ if( pwdlen > PKCS12_MAX_PWDLEN )
+ return( MBEDTLS_ERR_PKCS12_BAD_INPUT_DATA );
memset( &salt, 0, sizeof(mbedtls_asn1_buf) );
memset( &unipwd, 0, sizeof(unipwd) );
@@ -125,6 +130,8 @@
return( 0 );
}
+#undef PKCS12_MAX_PWDLEN
+
int mbedtls_pkcs12_pbe_sha1_rc4_128( mbedtls_asn1_buf *pbe_params, int mode,
const unsigned char *pwd, size_t pwdlen,
const unsigned char *data, size_t len,
diff --git a/library/ssl_cli.c b/library/ssl_cli.c
index c82e2e7..32eae0f 100644
--- a/library/ssl_cli.c
+++ b/library/ssl_cli.c
@@ -60,6 +60,7 @@
size_t *olen )
{
unsigned char *p = buf;
+ const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN;
size_t hostname_len;
*olen = 0;
@@ -72,6 +73,12 @@
hostname_len = strlen( ssl->hostname );
+ if( end < p || (size_t)( end - p ) < hostname_len + 9 )
+ {
+ MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
+ return;
+ }
+
/*
* struct {
* NameType name_type;
@@ -115,6 +122,7 @@
size_t *olen )
{
unsigned char *p = buf;
+ const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN;
*olen = 0;
@@ -123,6 +131,12 @@
MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding renegotiation extension" ) );
+ if( end < p || (size_t)( end - p ) < 5 + ssl->verify_data_len )
+ {
+ MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
+ return;
+ }
+
/*
* Secure renegotiation
*/
@@ -149,6 +163,7 @@
size_t *olen )
{
unsigned char *p = buf;
+ const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN;
size_t sig_alg_len = 0;
const int *md;
#if defined(MBEDTLS_RSA_C) || defined(MBEDTLS_ECDSA_C)
@@ -162,9 +177,27 @@
MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding signature_algorithms extension" ) );
+ for( md = ssl->conf->sig_hashes; *md != MBEDTLS_MD_NONE; md++ )
+ {
+#if defined(MBEDTLS_ECDSA_C)
+ sig_alg_len += 2;
+#endif
+#if defined(MBEDTLS_RSA_C)
+ sig_alg_len += 2;
+#endif
+ }
+
+ if( end < p || (size_t)( end - p ) < sig_alg_len + 6 )
+ {
+ MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
+ return;
+ }
+
/*
* Prepare signature_algorithms extension (TLS 1.2)
*/
+ sig_alg_len = 0;
+
for( md = ssl->conf->sig_hashes; *md != MBEDTLS_MD_NONE; md++ )
{
#if defined(MBEDTLS_ECDSA_C)
@@ -214,6 +247,7 @@
size_t *olen )
{
unsigned char *p = buf;
+ const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN;
unsigned char *elliptic_curve_list = p + 6;
size_t elliptic_curve_len = 0;
const mbedtls_ecp_curve_info *info;
@@ -235,6 +269,25 @@
for( info = mbedtls_ecp_curve_list(); info->grp_id != MBEDTLS_ECP_DP_NONE; info++ )
{
#endif
+ elliptic_curve_len += 2;
+ }
+
+ if( end < p || (size_t)( end - p ) < 6 + elliptic_curve_len )
+ {
+ MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
+ return;
+ }
+
+ elliptic_curve_len = 0;
+
+#if defined(MBEDTLS_ECP_C)
+ for( grp_id = ssl->conf->curve_list; *grp_id != MBEDTLS_ECP_DP_NONE; grp_id++ )
+ {
+ info = mbedtls_ecp_curve_info_from_grp_id( *grp_id );
+#else
+ for( info = mbedtls_ecp_curve_list(); info->grp_id != MBEDTLS_ECP_DP_NONE; info++ )
+ {
+#endif
elliptic_curve_list[elliptic_curve_len++] = info->tls_id >> 8;
elliptic_curve_list[elliptic_curve_len++] = info->tls_id & 0xFF;
@@ -260,12 +313,18 @@
size_t *olen )
{
unsigned char *p = buf;
- ((void) ssl);
+ const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN;
*olen = 0;
MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding supported_point_formats extension" ) );
+ if( end < p || (size_t)( end - p ) < 6 )
+ {
+ MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
+ return;
+ }
+
*p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_SUPPORTED_POINT_FORMATS >> 8 ) & 0xFF );
*p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_SUPPORTED_POINT_FORMATS ) & 0xFF );
@@ -285,14 +344,22 @@
size_t *olen )
{
unsigned char *p = buf;
+ const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN;
+
+ *olen = 0;
if( ssl->conf->mfl_code == MBEDTLS_SSL_MAX_FRAG_LEN_NONE ) {
- *olen = 0;
return;
}
MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding max_fragment_length extension" ) );
+ if( end < p || (size_t)( end - p ) < 5 )
+ {
+ MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
+ return;
+ }
+
*p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_MAX_FRAGMENT_LENGTH >> 8 ) & 0xFF );
*p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_MAX_FRAGMENT_LENGTH ) & 0xFF );
@@ -310,15 +377,23 @@
unsigned char *buf, size_t *olen )
{
unsigned char *p = buf;
+ const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN;
+
+ *olen = 0;
if( ssl->conf->trunc_hmac == MBEDTLS_SSL_TRUNC_HMAC_DISABLED )
{
- *olen = 0;
return;
}
MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding truncated_hmac extension" ) );
+ if( end < p || (size_t)( end - p ) < 4 )
+ {
+ MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
+ return;
+ }
+
*p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_TRUNCATED_HMAC >> 8 ) & 0xFF );
*p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_TRUNCATED_HMAC ) & 0xFF );
@@ -334,17 +409,25 @@
unsigned char *buf, size_t *olen )
{
unsigned char *p = buf;
+ const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN;
+
+ *olen = 0;
if( ssl->conf->encrypt_then_mac == MBEDTLS_SSL_ETM_DISABLED ||
ssl->conf->max_minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 )
{
- *olen = 0;
return;
}
MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding encrypt_then_mac "
"extension" ) );
+ if( end < p || (size_t)( end - p ) < 4 )
+ {
+ MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
+ return;
+ }
+
*p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_ENCRYPT_THEN_MAC >> 8 ) & 0xFF );
*p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_ENCRYPT_THEN_MAC ) & 0xFF );
@@ -360,17 +443,25 @@
unsigned char *buf, size_t *olen )
{
unsigned char *p = buf;
+ const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN;
+
+ *olen = 0;
if( ssl->conf->extended_ms == MBEDTLS_SSL_EXTENDED_MS_DISABLED ||
ssl->conf->max_minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 )
{
- *olen = 0;
return;
}
MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding extended_master_secret "
"extension" ) );
+ if( end < p || (size_t)( end - p ) < 4 )
+ {
+ MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
+ return;
+ }
+
*p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_EXTENDED_MASTER_SECRET >> 8 ) & 0xFF );
*p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_EXTENDED_MASTER_SECRET ) & 0xFF );
@@ -386,16 +477,24 @@
unsigned char *buf, size_t *olen )
{
unsigned char *p = buf;
+ const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN;
size_t tlen = ssl->session_negotiate->ticket_len;
+ *olen = 0;
+
if( ssl->conf->session_tickets == MBEDTLS_SSL_SESSION_TICKETS_DISABLED )
{
- *olen = 0;
return;
}
MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding session ticket extension" ) );
+ if( end < p || (size_t)( end - p ) < 4 + tlen )
+ {
+ MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
+ return;
+ }
+
*p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_SESSION_TICKET >> 8 ) & 0xFF );
*p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_SESSION_TICKET ) & 0xFF );
@@ -404,8 +503,7 @@
*olen = 4;
- if( ssl->session_negotiate->ticket == NULL ||
- ssl->session_negotiate->ticket_len == 0 )
+ if( ssl->session_negotiate->ticket == NULL || tlen == 0 )
{
return;
}
@@ -423,16 +521,28 @@
unsigned char *buf, size_t *olen )
{
unsigned char *p = buf;
+ const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_MAX_CONTENT_LEN;
+ size_t alpnlen = 0;
const char **cur;
+ *olen = 0;
+
if( ssl->conf->alpn_list == NULL )
{
- *olen = 0;
return;
}
MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding alpn extension" ) );
+ for( cur = ssl->conf->alpn_list; *cur != NULL; cur++ )
+ alpnlen += (unsigned char)( strlen( *cur ) & 0xFF ) + 1;
+
+ if( end < p || (size_t)( end - p ) < 6 + alpnlen )
+ {
+ MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
+ return;
+ }
+
*p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_ALPN >> 8 ) & 0xFF );
*p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_ALPN ) & 0xFF );
@@ -799,13 +909,13 @@
ext_len += olen;
#endif
-#if defined(MBEDTLS_SSL_SESSION_TICKETS)
- ssl_write_session_ticket_ext( ssl, p + 2 + ext_len, &olen );
+#if defined(MBEDTLS_SSL_ALPN)
+ ssl_write_alpn_ext( ssl, p + 2 + ext_len, &olen );
ext_len += olen;
#endif
-#if defined(MBEDTLS_SSL_ALPN)
- ssl_write_alpn_ext( ssl, p + 2 + ext_len, &olen );
+#if defined(MBEDTLS_SSL_SESSION_TICKETS)
+ ssl_write_session_ticket_ext( ssl, p + 2 + ext_len, &olen );
ext_len += olen;
#endif
diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index d9b05fd..43cbe0f 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -5730,7 +5730,7 @@
return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA );
if( ssl->handshake->psk != NULL )
- mbedtls_free( ssl->conf->psk );
+ mbedtls_free( ssl->handshake->psk );
if( ( ssl->handshake->psk = mbedtls_calloc( 1, psk_len ) ) == NULL )
{
@@ -5833,6 +5833,9 @@
if( hostname_len + 1 == 0 )
return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA );
+ if( hostname_len > MBEDTLS_SSL_MAX_HOST_NAME_LEN )
+ return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA );
+
ssl->hostname = mbedtls_calloc( 1, hostname_len + 1 );
if( ssl->hostname == NULL )
@@ -6993,7 +6996,7 @@
#endif
/*
- * Load default in mbetls_ssl_config
+ * Load default in mbedtls_ssl_config
*/
int mbedtls_ssl_config_defaults( mbedtls_ssl_config *conf,
int endpoint, int transport, int preset )