From 72f767da310045a31e15a2c2c5be80afc4102f79 Mon Sep 17 00:00:00 2001 From: Nadav0077 <18245584+Nadav0077@users.noreply.github.com> Date: Tue, 19 May 2026 14:15:02 +0300 Subject: [PATCH] Address review comments on serialized data hardening Signed-off-by: Nadav0077 <18245584+Nadav0077@users.noreply.github.com> --- library/ssl_tls.c | 10 ++++++---- tests/suites/test_suite_ssl.data | 6 +++--- tests/suites/test_suite_ssl.function | 6 ++++++ 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index cbb3959c6e..cb14275d1e 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3634,8 +3634,9 @@ static int ssl_tls13_session_load(mbedtls_ssl_session *session, } if (alpn_len > 0) { - if (p[alpn_len - 1] != '\0' || - memchr(p, '\0', alpn_len - 1) != NULL) { + /* 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; } @@ -3664,8 +3665,9 @@ static int ssl_tls13_session_load(mbedtls_ssl_session *session, return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; } if (hostname_len > 0) { - if (p[hostname_len - 1] != '\0' || - memchr(p, '\0', hostname_len - 1) != NULL) { + /* 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; } diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data index fd90f0606e..eec4902929 100644 --- a/tests/suites/test_suite_ssl.data +++ b/tests/suites/test_suite_ssl.data @@ -3029,15 +3029,15 @@ depends_on:MBEDTLS_SSL_PROTO_TLS1_3:MBEDTLS_SSL_SESSION_TICKETS:MBEDTLS_SSL_SRV_ 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_PROTO_TLS1_3:MBEDTLS_SSL_SESSION_TICKETS:MBEDTLS_SSL_CLI_C +depends_on:MBEDTLS_SSL_CLI_C ssl_tls13_session_load_rejects_bad_serialized_data:MBEDTLS_SSL_IS_CLIENT:0 TLS 1.3: Session serialization rejects hostname without terminator -depends_on:MBEDTLS_SSL_PROTO_TLS1_3:MBEDTLS_SSL_SESSION_TICKETS:MBEDTLS_SSL_CLI_C:MBEDTLS_SSL_SERVER_NAME_INDICATION +depends_on:MBEDTLS_SSL_CLI_C:MBEDTLS_SSL_SERVER_NAME_INDICATION ssl_tls13_session_load_rejects_bad_serialized_data:MBEDTLS_SSL_IS_CLIENT:1 TLS 1.3: Session serialization rejects ALPN without terminator -depends_on:MBEDTLS_SSL_PROTO_TLS1_3:MBEDTLS_SSL_SESSION_TICKETS:MBEDTLS_SSL_SRV_C:MBEDTLS_SSL_EARLY_DATA:MBEDTLS_SSL_ALPN +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:2 Test configuration of EC groups through mbedtls_ssl_conf_groups() diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index af63d6dbca..f5a047098a 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -2758,6 +2758,12 @@ void ssl_tls13_session_load_rejects_bad_serialized_data(int endpoint_type, 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 0: