diff --git a/ChangeLog.d/serialized-data-load-hardening.txt b/ChangeLog.d/serialized-data-load-hardening.txt new file mode 100644 index 0000000000..e068988dac --- /dev/null +++ b/ChangeLog.d/serialized-data-load-hardening.txt @@ -0,0 +1,5 @@ +Bugfix + * Reject malformed TLS 1.3 serialized sessions instead of accepting + unterminated strings or extra trailing data. + * Reject serialized SSL contexts whose DTLS Connection ID length exceeds + the maximum supported size, instead of overflowing the internal buffer. diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 88aaf85d29..fd67726d4d 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -4035,6 +4035,12 @@ static int ssl_tls13_session_load(mbedtls_ssl_session *session, } if (alpn_len > 0) { + /* The data is about to be used as a null-terminated string, so + * check that it actually is one. */ + if (p[alpn_len - 1] != '\0') { + return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; + } + int ret = mbedtls_ssl_session_set_ticket_alpn(session, (char *) p); if (ret != 0) { return ret; @@ -4060,11 +4066,16 @@ static int ssl_tls13_session_load(mbedtls_ssl_session *session, return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; } if (hostname_len > 0) { - session->hostname = mbedtls_calloc(1, hostname_len); - if (session->hostname == NULL) { - return MBEDTLS_ERR_SSL_ALLOC_FAILED; + /* The data is about to be used as a null-terminated string, so + * check that it actually is one. */ + if (p[hostname_len - 1] != '\0') { + return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; + } + + int ret = mbedtls_ssl_session_set_hostname(session, (const char *) p); + if (ret != 0) { + return ret; } - memcpy(session->hostname, p, hostname_len); p += hostname_len; } #endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ @@ -4102,6 +4113,10 @@ static int ssl_tls13_session_load(mbedtls_ssl_session *session, } #endif /* MBEDTLS_SSL_CLI_C */ + if (p != end) { + return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; + } + return 0; } @@ -5440,6 +5455,10 @@ static int ssl_context_load(mbedtls_ssl_context *ssl, return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; } + if (ssl->transform->in_cid_len > sizeof(ssl->transform->in_cid)) { + return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; + } + memcpy(ssl->transform->in_cid, p, ssl->transform->in_cid_len); p += ssl->transform->in_cid_len; @@ -5449,6 +5468,10 @@ static int ssl_context_load(mbedtls_ssl_context *ssl, return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; } + if (ssl->transform->out_cid_len > sizeof(ssl->transform->out_cid)) { + return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; + } + memcpy(ssl->transform->out_cid, p, ssl->transform->out_cid_len); p += ssl->transform->out_cid_len; #endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data index 06e8b62303..77a5ab2244 100644 --- a/tests/suites/test_suite_ssl.data +++ b/tests/suites/test_suite_ssl.data @@ -3185,6 +3185,19 @@ TLS 1.3: SRV: Session serialization, load buffer size depends_on:MBEDTLS_SSL_PROTO_TLS1_3:MBEDTLS_SSL_SESSION_TICKETS:MBEDTLS_SSL_SRV_C ssl_serialize_session_load_buf_size:0:"":MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_VERSION_TLS1_3 +TLS 1.3: Session serialization rejects trailing data +depends_on:MBEDTLS_SSL_CLI_C +ssl_tls13_session_load_rejects_bad_serialized_data:MBEDTLS_SSL_IS_CLIENT:TEST_TLS13_SESSION_LOAD_TRAILING_DATA + +TLS 1.3: Session serialization rejects hostname without terminator +depends_on:MBEDTLS_SSL_CLI_C:MBEDTLS_SSL_SERVER_NAME_INDICATION +ssl_tls13_session_load_rejects_bad_serialized_data:MBEDTLS_SSL_IS_CLIENT:TEST_TLS13_SESSION_LOAD_HOSTNAME_NO_NUL + +TLS 1.3: Session serialization rejects ALPN without terminator +depends_on:MBEDTLS_SSL_SRV_C:MBEDTLS_SSL_EARLY_DATA:MBEDTLS_SSL_ALPN +ssl_tls13_session_load_rejects_bad_serialized_data:MBEDTLS_SSL_IS_SERVER:TEST_TLS13_SESSION_LOAD_ALPN_NO_NUL + + Test configuration of EC groups through mbedtls_ssl_conf_curves() conf_curve: diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index ae7ab55366..c58231e5a1 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -22,6 +22,11 @@ #define TEST_EARLY_DATA_NO_INITIAL_ALPN 6 #define TEST_EARLY_DATA_NO_LATER_ALPN 7 +/* Mutations for ssl_tls13_session_load_rejects_bad_serialized_data */ +#define TEST_TLS13_SESSION_LOAD_TRAILING_DATA 0 +#define TEST_TLS13_SESSION_LOAD_HOSTNAME_NO_NUL 1 +#define TEST_TLS13_SESSION_LOAD_ALPN_NO_NUL 2 + #if (!defined(MBEDTLS_SSL_PROTO_TLS1_2)) && \ defined(MBEDTLS_SSL_EARLY_DATA) && defined(MBEDTLS_SSL_CLI_C) && \ defined(MBEDTLS_SSL_SRV_C) && defined(MBEDTLS_DEBUG_C) && \ @@ -2701,6 +2706,80 @@ exit: } /* END_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_SSL_PROTO_TLS1_3:MBEDTLS_SSL_SESSION_TICKETS */ +void ssl_tls13_session_load_rejects_bad_serialized_data(int endpoint_type, + int mutation) +{ + mbedtls_ssl_session session, restored; + unsigned char *buf = NULL; + size_t len, bad_len, i; + unsigned char *field = NULL; + const unsigned char hostname[] = "hostname example"; + const unsigned char alpn[] = "ALPNExample"; + const unsigned char *string_to_corrupt = NULL; + size_t string_len = 0; + + mbedtls_ssl_session_init(&session); + mbedtls_ssl_session_init(&restored); + USE_PSA_INIT(); + + TEST_EQUAL(mbedtls_test_ssl_tls13_populate_session( + &session, 42, endpoint_type), 0); + + TEST_EQUAL(mbedtls_ssl_session_save(&session, NULL, 0, &len), + MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL); + TEST_CALLOC(buf, len + 1); + TEST_EQUAL(mbedtls_ssl_session_save(&session, buf, len, &len), 0); + + /* The serialized data must load successfully before we corrupt it, + * otherwise the test below would not prove anything. */ + TEST_EQUAL(mbedtls_ssl_session_load(&restored, buf, len), 0); + mbedtls_ssl_session_free(&restored); + mbedtls_ssl_session_init(&restored); + + bad_len = len; + switch (mutation) { + case TEST_TLS13_SESSION_LOAD_TRAILING_DATA: + bad_len = len + 1; + buf[len] = 0; + break; + + case TEST_TLS13_SESSION_LOAD_HOSTNAME_NO_NUL: + string_to_corrupt = hostname; + string_len = sizeof(hostname); + break; + + case TEST_TLS13_SESSION_LOAD_ALPN_NO_NUL: + string_to_corrupt = alpn; + string_len = sizeof(alpn); + break; + + default: + TEST_ASSERT(0); + } + + if (string_to_corrupt != NULL) { + for (i = 0; i + string_len <= len; i++) { + if (memcmp(buf + i, string_to_corrupt, string_len) == 0) { + field = buf + i; + break; + } + } + TEST_ASSERT(field != NULL); + field[string_len - 1] = 'X'; + } + + TEST_EQUAL(mbedtls_ssl_session_load(&restored, buf, bad_len), + MBEDTLS_ERR_SSL_BAD_INPUT_DATA); + +exit: + mbedtls_ssl_session_free(&session); + mbedtls_ssl_session_free(&restored); + mbedtls_free(buf); + USE_PSA_DONE(); +} +/* END_CASE */ + /* BEGIN_CASE */ void ssl_session_serialize_version_check(int corrupt_major, int corrupt_minor,