From b0d5c0c80a09b7e475fab4574fc140bc3ed18934 Mon Sep 17 00:00:00 2001 From: Nadav0077 <18245584+Nadav0077@users.noreply.github.com> Date: Fri, 22 May 2026 13:16:58 +0300 Subject: [PATCH] Harden serialized data deserialization Reject malformed TLS 1.3 serialized sessions instead of accepting unterminated strings or extra trailing data, and reject serialized SSL contexts whose DTLS Connection ID length exceeds the maximum supported size instead of overflowing the internal buffer. Backport of #10715 (development). The dedicated CID-length test from #10715 relies on test helpers that do not exist on 3.6 (notably mbedtls_test_ssl_dtls_join_endpoints and mbedtls_test_ssl_perform_connection) and is therefore omitted from this backport; the production fix and the TLS 1.3 session-load tests are backported unchanged. Signed-off-by: Nadav0077 <18245584+Nadav0077@users.noreply.github.com> --- .../serialized-data-load-hardening.txt | 5 ++ library/ssl_tls.c | 31 +++++++- tests/suites/test_suite_ssl.data | 13 +++ tests/suites/test_suite_ssl.function | 79 +++++++++++++++++++ 4 files changed, 124 insertions(+), 4 deletions(-) create mode 100644 ChangeLog.d/serialized-data-load-hardening.txt 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,