From 0d5f414adee430ac7a78e97b3de0fec609b2cb6a Mon Sep 17 00:00:00 2001 From: Naveed Date: Thu, 25 Jun 2026 13:05:46 +0530 Subject: [PATCH 1/6] Reject out-of-bounds ALPN length in ssl_context_load The serialized ALPN length was passed to memcmp() against each configured protocol without checking it against the remaining serialized data, so a malformed context could read past the end of the buffer. Bound it before the comparison, like the adjacent CID fields. Signed-off-by: Naveed --- ChangeLog.d/ssl-context-load-alpn-bounds.txt | 4 + library/ssl_tls.c | 4 + tests/suites/test_suite_ssl.data | 4 + tests/suites/test_suite_ssl.function | 83 ++++++++++++++++++++ 4 files changed, 95 insertions(+) create mode 100644 ChangeLog.d/ssl-context-load-alpn-bounds.txt diff --git a/ChangeLog.d/ssl-context-load-alpn-bounds.txt b/ChangeLog.d/ssl-context-load-alpn-bounds.txt new file mode 100644 index 0000000000..77f21d6834 --- /dev/null +++ b/ChangeLog.d/ssl-context-load-alpn-bounds.txt @@ -0,0 +1,4 @@ +Bugfix + * Reject serialized SSL contexts whose ALPN protocol length exceeds the + remaining serialized data, instead of reading past the end of the buffer + in mbedtls_ssl_context_load(). diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 0195576213..d7273ee0de 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -5098,6 +5098,10 @@ static int ssl_context_load(mbedtls_ssl_context *ssl, alpn_len = *p++; + if ((size_t) (end - p) < alpn_len) { + return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; + } + if (alpn_len != 0 && ssl->conf->alpn_list != NULL) { /* alpn_chosen should point to an item in the configured list */ for (cur = ssl->conf->alpn_list; *cur != NULL; cur++) { diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data index f7876d993e..3d15030bc9 100644 --- a/tests/suites/test_suite_ssl.data +++ b/tests/suites/test_suite_ssl.data @@ -3058,6 +3058,10 @@ SSL context serialization rejects out-of-bounds DTLS CID length depends_on:MBEDTLS_SSL_HANDSHAKE_WITH_CERT_ENABLED:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_SSL_PROTO_DTLS:MBEDTLS_SSL_DTLS_CONNECTION_ID:MBEDTLS_SSL_CONTEXT_SERIALIZATION:PSA_HAVE_ALG_SOME_RSA_SIGN:PSA_WANT_ECC_SECP_R1_384:PSA_WANT_ALG_SHA_256:MBEDTLS_CAN_HANDLE_RSA_TEST_KEY:TEST_GCM_OR_CHACHAPOLY_ENABLED ssl_context_load_rejects_oob_cid_length: +SSL context serialization rejects out-of-bounds ALPN length +depends_on:MBEDTLS_SSL_HANDSHAKE_WITH_CERT_ENABLED:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_SSL_PROTO_DTLS:MBEDTLS_SSL_ALPN:MBEDTLS_SSL_CONTEXT_SERIALIZATION:PSA_HAVE_ALG_SOME_RSA_SIGN:PSA_WANT_ECC_SECP_R1_384:PSA_WANT_ALG_SHA_256:MBEDTLS_CAN_HANDLE_RSA_TEST_KEY:TEST_GCM_OR_CHACHAPOLY_ENABLED +ssl_context_load_rejects_oob_alpn_length: + Test configuration of EC groups through mbedtls_ssl_conf_groups() conf_group: diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index c1cf9bab86..9d2ecf6484 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -3082,6 +3082,89 @@ exit: } /* END_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_SSL_HANDSHAKE_WITH_CERT_ENABLED:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_SSL_PROTO_DTLS:MBEDTLS_SSL_ALPN:MBEDTLS_SSL_CONTEXT_SERIALIZATION:PSA_HAVE_ALG_SOME_RSA_SIGN:PSA_WANT_ECC_SECP_R1_384:PSA_WANT_ALG_SHA_256:MBEDTLS_CAN_HANDLE_RSA_TEST_KEY:TEST_GCM_OR_CHACHAPOLY_ENABLED */ +void ssl_context_load_rejects_oob_alpn_length(void) +{ + /* The negotiated protocol (alpn[0]) is serialized; alpn[1] is longer and is + * what the corrupted length will match against on load, so the unbounded + * memcmp() reads past the end of the buffer. */ + const char negotiated[] = "alpnsentinel"; + const char *alpn[] = { negotiated, "alpnsentinel-padding", NULL }; + + mbedtls_test_handshake_test_options options; + mbedtls_test_ssl_endpoint client_ep, server_ep; + unsigned char *context_buf = NULL; + size_t context_buf_len, i; + unsigned char *alpn_len_p = NULL; + + memset(&client_ep, 0, sizeof(client_ep)); + memset(&server_ep, 0, sizeof(server_ep)); + + MD_OR_USE_PSA_INIT(); + mbedtls_test_init_handshake_options(&options); + + options.dtls = 1; + options.expected_negotiated_version = MBEDTLS_SSL_VERSION_TLS1_2; + + TEST_EQUAL(mbedtls_test_ssl_endpoint_init( + &client_ep, MBEDTLS_SSL_IS_CLIENT, &options), 0); + TEST_EQUAL(mbedtls_test_ssl_endpoint_init( + &server_ep, MBEDTLS_SSL_IS_SERVER, &options), 0); + + /* Offer the same protocol list on both peers so alpn[0] is negotiated and + * serialized into the context. */ + TEST_EQUAL(mbedtls_ssl_conf_alpn_protocols(&client_ep.conf, alpn), 0); + TEST_EQUAL(mbedtls_ssl_conf_alpn_protocols(&server_ep.conf, alpn), 0); + + TEST_EQUAL(mbedtls_test_ssl_dtls_join_endpoints(&client_ep, &server_ep), 0); + + TEST_ASSERT(mbedtls_test_ssl_perform_connection(&options, &client_ep, + &server_ep)); + + TEST_ASSERT(server_ep.ssl.alpn_chosen != NULL); + TEST_EQUAL(strcmp(server_ep.ssl.alpn_chosen, negotiated), 0); + + /* Serialize the post-handshake server context. */ + TEST_EQUAL(mbedtls_ssl_context_save(&server_ep.ssl, NULL, 0, + &context_buf_len), + MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL); + TEST_CALLOC(context_buf, context_buf_len); + TEST_EQUAL(mbedtls_ssl_context_save(&server_ep.ssl, context_buf, + context_buf_len, &context_buf_len), 0); + + /* The serialized format writes [alpn_len][alpn bytes]; locate the length + * byte by searching for the negotiated protocol string. */ + for (i = 1; i + strlen(negotiated) <= context_buf_len; i++) { + if (context_buf[i - 1] == (unsigned char) strlen(negotiated) && + memcmp(context_buf + i, negotiated, strlen(negotiated)) == 0) { + alpn_len_p = context_buf + i - 1; + break; + } + } + TEST_ASSERT(alpn_len_p != NULL); + + /* Claim the length of the longer configured protocol, which is more bytes + * than remain in the buffer. */ + *alpn_len_p = (unsigned char) strlen(alpn[1]); + + /* Reinitialise the server SSL context so we can load into a fresh one. */ + mbedtls_ssl_free(&server_ep.ssl); + mbedtls_ssl_init(&server_ep.ssl); + TEST_EQUAL(mbedtls_ssl_setup(&server_ep.ssl, &server_ep.conf), 0); + + TEST_EQUAL(mbedtls_ssl_context_load(&server_ep.ssl, context_buf, + context_buf_len), + MBEDTLS_ERR_SSL_BAD_INPUT_DATA); + +exit: + mbedtls_test_ssl_endpoint_free(&client_ep); + mbedtls_test_ssl_endpoint_free(&server_ep); + mbedtls_test_free_handshake_options(&options); + mbedtls_free(context_buf); + MD_OR_USE_PSA_DONE(); +} +/* END_CASE */ + /* BEGIN_CASE */ void ssl_session_serialize_version_check(int corrupt_major, int corrupt_minor, From 995a714843fa590a6da4054cfc7f86a30d29a71e Mon Sep 17 00:00:00 2001 From: Naveed Date: Thu, 9 Jul 2026 13:17:05 +0530 Subject: [PATCH 2/6] Merge the two ssl_context_load OOB length tests Fold ssl_context_load_rejects_oob_cid_length and ssl_context_load_rejects_oob_alpn_length into a single parameterized test function, since they shared most of their code. Signed-off-by: Naveed --- tests/suites/test_suite_ssl.data | 8 +- tests/suites/test_suite_ssl.function | 174 +++++++++++---------------- 2 files changed, 76 insertions(+), 106 deletions(-) diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data index 3d15030bc9..db25e5417d 100644 --- a/tests/suites/test_suite_ssl.data +++ b/tests/suites/test_suite_ssl.data @@ -3055,12 +3055,12 @@ 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 SSL context serialization rejects out-of-bounds DTLS CID length -depends_on:MBEDTLS_SSL_HANDSHAKE_WITH_CERT_ENABLED:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_SSL_PROTO_DTLS:MBEDTLS_SSL_DTLS_CONNECTION_ID:MBEDTLS_SSL_CONTEXT_SERIALIZATION:PSA_HAVE_ALG_SOME_RSA_SIGN:PSA_WANT_ECC_SECP_R1_384:PSA_WANT_ALG_SHA_256:MBEDTLS_CAN_HANDLE_RSA_TEST_KEY:TEST_GCM_OR_CHACHAPOLY_ENABLED -ssl_context_load_rejects_oob_cid_length: +depends_on:MBEDTLS_SSL_DTLS_CONNECTION_ID +ssl_context_load_rejects_oob_length:TEST_SSL_CONTEXT_LOAD_OOB_CID SSL context serialization rejects out-of-bounds ALPN length -depends_on:MBEDTLS_SSL_HANDSHAKE_WITH_CERT_ENABLED:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_SSL_PROTO_DTLS:MBEDTLS_SSL_ALPN:MBEDTLS_SSL_CONTEXT_SERIALIZATION:PSA_HAVE_ALG_SOME_RSA_SIGN:PSA_WANT_ECC_SECP_R1_384:PSA_WANT_ALG_SHA_256:MBEDTLS_CAN_HANDLE_RSA_TEST_KEY:TEST_GCM_OR_CHACHAPOLY_ENABLED -ssl_context_load_rejects_oob_alpn_length: +depends_on:MBEDTLS_SSL_ALPN +ssl_context_load_rejects_oob_length:TEST_SSL_CONTEXT_LOAD_OOB_ALPN Test configuration of EC groups through mbedtls_ssl_conf_groups() conf_group: diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 9d2ecf6484..ed60427094 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -31,6 +31,10 @@ #define TEST_TLS13_SESSION_LOAD_HOSTNAME_NO_NUL 1 #define TEST_TLS13_SESSION_LOAD_ALPN_NO_NUL 2 +/* Corrupted length fields for ssl_context_load_rejects_oob_length */ +#define TEST_SSL_CONTEXT_LOAD_OOB_CID 0 +#define TEST_SSL_CONTEXT_LOAD_OOB_ALPN 1 + #if defined(MBEDTLS_SSL_PROTO_TLS1_2) && \ defined(MBEDTLS_SSL_SESSION_TICKETS) && \ defined(MBEDTLS_SSL_CLI_C) && defined(MBEDTLS_SSL_SRV_C) && \ @@ -2992,9 +2996,10 @@ exit: } /* END_CASE */ -/* BEGIN_CASE depends_on:MBEDTLS_SSL_HANDSHAKE_WITH_CERT_ENABLED:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_SSL_PROTO_DTLS:MBEDTLS_SSL_DTLS_CONNECTION_ID:MBEDTLS_SSL_CONTEXT_SERIALIZATION:PSA_HAVE_ALG_SOME_RSA_SIGN:PSA_WANT_ECC_SECP_R1_384:PSA_WANT_ALG_SHA_256:MBEDTLS_CAN_HANDLE_RSA_TEST_KEY:TEST_GCM_OR_CHACHAPOLY_ENABLED */ -void ssl_context_load_rejects_oob_cid_length(void) +/* BEGIN_CASE depends_on:MBEDTLS_SSL_HANDSHAKE_WITH_CERT_ENABLED:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_SSL_PROTO_DTLS:MBEDTLS_SSL_CONTEXT_SERIALIZATION:PSA_HAVE_ALG_SOME_RSA_SIGN:PSA_WANT_ECC_SECP_R1_384:PSA_WANT_ALG_SHA_256:MBEDTLS_CAN_HANDLE_RSA_TEST_KEY:TEST_GCM_OR_CHACHAPOLY_ENABLED */ +void ssl_context_load_rejects_oob_length(int field) { +#if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) /* Sentinel CID so we can find its location inside the serialized buffer. */ const unsigned char server_cid[] = { 0xAB, 0xCD, 0xEF, 0x12, 0x34, 0x56, 0x78, 0x9A @@ -3002,100 +3007,23 @@ void ssl_context_load_rejects_oob_cid_length(void) const unsigned char client_cid[] = { 0x9A, 0x78, 0x56, 0x34, 0x12, 0xEF, 0xCD, 0xAB }; - - mbedtls_test_handshake_test_options options; - mbedtls_test_ssl_endpoint client_ep, server_ep; - unsigned char *context_buf = NULL; - size_t context_buf_len, i; - unsigned char *cid_len_p = NULL; - - memset(&client_ep, 0, sizeof(client_ep)); - memset(&server_ep, 0, sizeof(server_ep)); - - MD_OR_USE_PSA_INIT(); - mbedtls_test_init_handshake_options(&options); - - options.dtls = 1; - options.expected_negotiated_version = MBEDTLS_SSL_VERSION_TLS1_2; - - TEST_EQUAL(mbedtls_test_ssl_endpoint_init( - &client_ep, MBEDTLS_SSL_IS_CLIENT, &options), 0); - TEST_EQUAL(mbedtls_test_ssl_endpoint_init( - &server_ep, MBEDTLS_SSL_IS_SERVER, &options), 0); - - /* Configure CID with unique patterns so we can locate the length byte in - * the serialized context. Both peers must agree on the CID length via - * conf_cid() before set_cid() will accept the value. */ - TEST_EQUAL(mbedtls_ssl_conf_cid(&client_ep.conf, sizeof(client_cid), - MBEDTLS_SSL_UNEXPECTED_CID_FAIL), 0); - TEST_EQUAL(mbedtls_ssl_conf_cid(&server_ep.conf, sizeof(server_cid), - MBEDTLS_SSL_UNEXPECTED_CID_FAIL), 0); - TEST_EQUAL(mbedtls_ssl_set_cid(&client_ep.ssl, MBEDTLS_SSL_CID_ENABLED, - client_cid, sizeof(client_cid)), 0); - TEST_EQUAL(mbedtls_ssl_set_cid(&server_ep.ssl, MBEDTLS_SSL_CID_ENABLED, - server_cid, sizeof(server_cid)), 0); - - TEST_EQUAL(mbedtls_test_ssl_dtls_join_endpoints(&client_ep, &server_ep), 0); - - TEST_ASSERT(mbedtls_test_ssl_perform_connection(&options, &client_ep, - &server_ep)); - - /* Serialize the post-handshake server context. */ - TEST_EQUAL(mbedtls_ssl_context_save(&server_ep.ssl, NULL, 0, - &context_buf_len), - MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL); - TEST_CALLOC(context_buf, context_buf_len); - TEST_EQUAL(mbedtls_ssl_context_save(&server_ep.ssl, context_buf, - context_buf_len, &context_buf_len), 0); - - /* The serialized format writes [in_cid_len][in_cid bytes]; locate the - * length byte by searching for our sentinel CID. */ - for (i = 1; i + sizeof(server_cid) <= context_buf_len; i++) { - if (context_buf[i - 1] == (unsigned char) sizeof(server_cid) && - memcmp(context_buf + i, server_cid, sizeof(server_cid)) == 0) { - cid_len_p = context_buf + i - 1; - break; - } - } - TEST_ASSERT(cid_len_p != NULL); - - /* Corrupt the length to the smallest value strictly greater than - * sizeof(in_cid), so we exercise the new bounds check rather than the - * pre-existing buffer-size check that would also trip on huge values. */ - *cid_len_p = (unsigned char) (MBEDTLS_SSL_CID_IN_LEN_MAX + 1); - - /* Reinitialise the server SSL context so we can load into a fresh one. */ - mbedtls_ssl_free(&server_ep.ssl); - mbedtls_ssl_init(&server_ep.ssl); - TEST_EQUAL(mbedtls_ssl_setup(&server_ep.ssl, &server_ep.conf), 0); - - TEST_EQUAL(mbedtls_ssl_context_load(&server_ep.ssl, context_buf, - context_buf_len), - MBEDTLS_ERR_SSL_BAD_INPUT_DATA); - -exit: - mbedtls_test_ssl_endpoint_free(&client_ep); - mbedtls_test_ssl_endpoint_free(&server_ep); - mbedtls_test_free_handshake_options(&options); - mbedtls_free(context_buf); - MD_OR_USE_PSA_DONE(); -} -/* END_CASE */ - -/* BEGIN_CASE depends_on:MBEDTLS_SSL_HANDSHAKE_WITH_CERT_ENABLED:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_SSL_PROTO_DTLS:MBEDTLS_SSL_ALPN:MBEDTLS_SSL_CONTEXT_SERIALIZATION:PSA_HAVE_ALG_SOME_RSA_SIGN:PSA_WANT_ECC_SECP_R1_384:PSA_WANT_ALG_SHA_256:MBEDTLS_CAN_HANDLE_RSA_TEST_KEY:TEST_GCM_OR_CHACHAPOLY_ENABLED */ -void ssl_context_load_rejects_oob_alpn_length(void) -{ +#endif +#if defined(MBEDTLS_SSL_ALPN) /* The negotiated protocol (alpn[0]) is serialized; alpn[1] is longer and is * what the corrupted length will match against on load, so the unbounded * memcmp() reads past the end of the buffer. */ const char negotiated[] = "alpnsentinel"; const char *alpn[] = { negotiated, "alpnsentinel-padding", NULL }; +#endif mbedtls_test_handshake_test_options options; mbedtls_test_ssl_endpoint client_ep, server_ep; unsigned char *context_buf = NULL; size_t context_buf_len, i; - unsigned char *alpn_len_p = NULL; + const unsigned char *sentinel = NULL; + size_t sentinel_len = 0; + unsigned char corrupt_len = 0; + unsigned char *len_p = NULL; memset(&client_ep, 0, sizeof(client_ep)); memset(&server_ep, 0, sizeof(server_ep)); @@ -3111,18 +3039,62 @@ void ssl_context_load_rejects_oob_alpn_length(void) TEST_EQUAL(mbedtls_test_ssl_endpoint_init( &server_ep, MBEDTLS_SSL_IS_SERVER, &options), 0); - /* Offer the same protocol list on both peers so alpn[0] is negotiated and - * serialized into the context. */ - TEST_EQUAL(mbedtls_ssl_conf_alpn_protocols(&client_ep.conf, alpn), 0); - TEST_EQUAL(mbedtls_ssl_conf_alpn_protocols(&server_ep.conf, alpn), 0); + switch (field) { +#if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) + case TEST_SSL_CONTEXT_LOAD_OOB_CID: + /* Configure CID with unique patterns so we can locate the length + * byte in the serialized context. Both peers must agree on the CID + * length via conf_cid() before set_cid() will accept the value. */ + TEST_EQUAL(mbedtls_ssl_conf_cid( + &client_ep.conf, sizeof(client_cid), + MBEDTLS_SSL_UNEXPECTED_CID_FAIL), 0); + TEST_EQUAL(mbedtls_ssl_conf_cid( + &server_ep.conf, sizeof(server_cid), + MBEDTLS_SSL_UNEXPECTED_CID_FAIL), 0); + TEST_EQUAL(mbedtls_ssl_set_cid( + &client_ep.ssl, MBEDTLS_SSL_CID_ENABLED, + client_cid, sizeof(client_cid)), 0); + TEST_EQUAL(mbedtls_ssl_set_cid( + &server_ep.ssl, MBEDTLS_SSL_CID_ENABLED, + server_cid, sizeof(server_cid)), 0); + sentinel = server_cid; + sentinel_len = sizeof(server_cid); + /* The smallest value strictly greater than sizeof(in_cid), so we + * exercise the bounds check rather than the pre-existing + * buffer-size check that would also trip on huge values. */ + corrupt_len = (unsigned char) (MBEDTLS_SSL_CID_IN_LEN_MAX + 1); + break; +#endif +#if defined(MBEDTLS_SSL_ALPN) + case TEST_SSL_CONTEXT_LOAD_OOB_ALPN: + /* Offer the same protocol list on both peers so alpn[0] is + * negotiated and serialized into the context. */ + TEST_EQUAL(mbedtls_ssl_conf_alpn_protocols(&client_ep.conf, alpn), + 0); + TEST_EQUAL(mbedtls_ssl_conf_alpn_protocols(&server_ep.conf, alpn), + 0); + sentinel = (const unsigned char *) negotiated; + sentinel_len = strlen(negotiated); + /* The length of the longer configured protocol, which is more + * bytes than remain in the buffer. */ + corrupt_len = (unsigned char) strlen(alpn[1]); + break; +#endif + default: + TEST_ASSERT(0); + } TEST_EQUAL(mbedtls_test_ssl_dtls_join_endpoints(&client_ep, &server_ep), 0); TEST_ASSERT(mbedtls_test_ssl_perform_connection(&options, &client_ep, &server_ep)); - TEST_ASSERT(server_ep.ssl.alpn_chosen != NULL); - TEST_EQUAL(strcmp(server_ep.ssl.alpn_chosen, negotiated), 0); +#if defined(MBEDTLS_SSL_ALPN) + if (field == TEST_SSL_CONTEXT_LOAD_OOB_ALPN) { + TEST_ASSERT(server_ep.ssl.alpn_chosen != NULL); + TEST_EQUAL(strcmp(server_ep.ssl.alpn_chosen, negotiated), 0); + } +#endif /* Serialize the post-handshake server context. */ TEST_EQUAL(mbedtls_ssl_context_save(&server_ep.ssl, NULL, 0, @@ -3132,20 +3104,18 @@ void ssl_context_load_rejects_oob_alpn_length(void) TEST_EQUAL(mbedtls_ssl_context_save(&server_ep.ssl, context_buf, context_buf_len, &context_buf_len), 0); - /* The serialized format writes [alpn_len][alpn bytes]; locate the length - * byte by searching for the negotiated protocol string. */ - for (i = 1; i + strlen(negotiated) <= context_buf_len; i++) { - if (context_buf[i - 1] == (unsigned char) strlen(negotiated) && - memcmp(context_buf + i, negotiated, strlen(negotiated)) == 0) { - alpn_len_p = context_buf + i - 1; + /* Both fields are serialized as [length byte][data bytes]; locate the + * length byte by searching for the sentinel data. */ + for (i = 1; i + sentinel_len <= context_buf_len; i++) { + if (context_buf[i - 1] == (unsigned char) sentinel_len && + memcmp(context_buf + i, sentinel, sentinel_len) == 0) { + len_p = context_buf + i - 1; break; } } - TEST_ASSERT(alpn_len_p != NULL); + TEST_ASSERT(len_p != NULL); - /* Claim the length of the longer configured protocol, which is more bytes - * than remain in the buffer. */ - *alpn_len_p = (unsigned char) strlen(alpn[1]); + *len_p = corrupt_len; /* Reinitialise the server SSL context so we can load into a fresh one. */ mbedtls_ssl_free(&server_ep.ssl); From 42e227f411a70b2f54a25ee8d62b1e9b68842614 Mon Sep 17 00:00:00 2001 From: Naveed Date: Sat, 11 Jul 2026 10:55:10 +0530 Subject: [PATCH 3/6] Address review: verify pristine load, corrupt ALPN length by exactly one Load the serialized context once before corrupting it, to check the data is well-formed to begin with. Shorten the second ALPN protocol so the corrupted length overruns the serialized data by exactly one byte, and assert that property at runtime. Document that the corrupted load usually fails even without the bounds check, so the regression is only caught when the suite runs under ASan or Valgrind. Signed-off-by: Naveed --- tests/suites/test_suite_ssl.function | 42 ++++++++++++++++++++++++---- 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index ed60427094..827bda6aa0 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -3009,11 +3009,17 @@ void ssl_context_load_rejects_oob_length(int field) }; #endif #if defined(MBEDTLS_SSL_ALPN) - /* The negotiated protocol (alpn[0]) is serialized; alpn[1] is longer and is - * what the corrupted length will match against on load, so the unbounded - * memcmp() reads past the end of the buffer. */ + /* The negotiated protocol (alpn[0]) is the last field in the serialized + * context, so exactly strlen(alpn[0]) bytes remain after its length byte. + * On load, ssl_context_load() only memcmp()s configured protocols whose + * strlen() equals the length byte, so the corrupted length has to match a + * configured protocol to reach the memcmp(): alpn[1] is one character + * longer than alpn[0], which makes the corrupted length point exactly one + * byte past the end of the serialized data. It shares alpn[0]'s prefix so + * that all in-bounds bytes match and the memcmp() cannot stop before + * reading past the end of the buffer. */ const char negotiated[] = "alpnsentinel"; - const char *alpn[] = { negotiated, "alpnsentinel-padding", NULL }; + const char *alpn[] = { negotiated, "alpnsentinel2", NULL }; #endif mbedtls_test_handshake_test_options options; @@ -3075,8 +3081,8 @@ void ssl_context_load_rejects_oob_length(int field) 0); sentinel = (const unsigned char *) negotiated; sentinel_len = strlen(negotiated); - /* The length of the longer configured protocol, which is more - * bytes than remain in the buffer. */ + /* One byte past the end of the serialized data; see the + * declaration of alpn[] above. */ corrupt_len = (unsigned char) strlen(alpn[1]); break; #endif @@ -3104,6 +3110,14 @@ void ssl_context_load_rejects_oob_length(int field) TEST_EQUAL(mbedtls_ssl_context_save(&server_ep.ssl, context_buf, context_buf_len, &context_buf_len), 0); + /* Double-check that the serialized data loads cleanly before it gets + * corrupted. */ + mbedtls_ssl_free(&server_ep.ssl); + mbedtls_ssl_init(&server_ep.ssl); + TEST_EQUAL(mbedtls_ssl_setup(&server_ep.ssl, &server_ep.conf), 0); + TEST_EQUAL(mbedtls_ssl_context_load(&server_ep.ssl, context_buf, + context_buf_len), 0); + /* Both fields are serialized as [length byte][data bytes]; locate the * length byte by searching for the sentinel data. */ for (i = 1; i + sentinel_len <= context_buf_len; i++) { @@ -3115,6 +3129,15 @@ void ssl_context_load_rejects_oob_length(int field) } TEST_ASSERT(len_p != NULL); +#if defined(MBEDTLS_SSL_ALPN) + if (field == TEST_SSL_CONTEXT_LOAD_OOB_ALPN) { + /* Double-check that the corrupted length overruns the serialized data + * by exactly one byte, as the ALPN field is the last one saved. */ + TEST_EQUAL((size_t) corrupt_len, + (size_t) (context_buf + context_buf_len - (len_p + 1)) + 1); + } +#endif + *len_p = corrupt_len; /* Reinitialise the server SSL context so we can load into a fresh one. */ @@ -3122,6 +3145,13 @@ void ssl_context_load_rejects_oob_length(int field) mbedtls_ssl_init(&server_ep.ssl); TEST_EQUAL(mbedtls_ssl_setup(&server_ep.ssl, &server_ep.conf), 0); + /* Note that this rejection alone does not prove the bounds check works: + * without it the ALPN case usually still fails with + * MBEDTLS_ERR_SSL_BAD_INPUT_DATA, because the out-of-bounds bytes are + * unlikely to match a configured protocol so the alpn_chosen lookup + * fails afterwards. What the missing check leaks is the out-of-bounds + * read in memcmp() itself, so this test case only catches a regression + * when the test suite runs under ASan or Valgrind. */ TEST_EQUAL(mbedtls_ssl_context_load(&server_ep.ssl, context_buf, context_buf_len), MBEDTLS_ERR_SSL_BAD_INPUT_DATA); From a3b63e16474dfc663510b7af37176ec40a52b6cc Mon Sep 17 00:00:00 2001 From: Naveed Date: Fri, 17 Jul 2026 14:52:08 +0530 Subject: [PATCH 4/6] Fix the CID case of the OOB length regression test The corrupted in_cid length exercised the pre-existing size-cap check, not the bounds check: the in_cid field is not the last one serialized, and the MBEDTLS_SSL_CID_IN_LEN_MAX cap is smaller than the size of the fields that follow it, so no corrupted length could reach past the end of the full buffer, and reverting the bounds check did not change the test's outcome. Truncate the loaded data right after the in_cid bytes, load from a tightly allocated copy, and corrupt the length to one byte past that end, so that reverting the bounds check now trips ASan with an out-of-bounds read in the memcpy(), like the ALPN case. Signed-off-by: Naveed --- tests/suites/test_suite_ssl.function | 60 ++++++++++++++++++---------- 1 file changed, 40 insertions(+), 20 deletions(-) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 827bda6aa0..185d0404f4 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -3025,7 +3025,8 @@ void ssl_context_load_rejects_oob_length(int field) mbedtls_test_handshake_test_options options; mbedtls_test_ssl_endpoint client_ep, server_ep; unsigned char *context_buf = NULL; - size_t context_buf_len, i; + unsigned char *load_buf = NULL; + size_t context_buf_len, load_len, i; const unsigned char *sentinel = NULL; size_t sentinel_len = 0; unsigned char corrupt_len = 0; @@ -3065,10 +3066,16 @@ void ssl_context_load_rejects_oob_length(int field) server_cid, sizeof(server_cid)), 0); sentinel = server_cid; sentinel_len = sizeof(server_cid); - /* The smallest value strictly greater than sizeof(in_cid), so we - * exercise the bounds check rather than the pre-existing - * buffer-size check that would also trip on huge values. */ - corrupt_len = (unsigned char) (MBEDTLS_SSL_CID_IN_LEN_MAX + 1); + /* The in_cid field sits in the middle of the serialized data and + * its length is capped at MBEDTLS_SSL_CID_IN_LEN_MAX by a + * separate check, which is less than the size of the fields that + * follow it, so no corrupted length can reach past the end of + * the full buffer. Instead the loaded data is truncated right + * after the in_cid bytes (see the computation of load_len below) + * and the length is corrupted to reach one byte past that end. + * Staying under the cap ensures the bounds check, not the cap, + * is what must reject the corrupted length. */ + corrupt_len = (unsigned char) (sizeof(server_cid) + 1); break; #endif #if defined(MBEDTLS_SSL_ALPN) @@ -3129,31 +3136,43 @@ void ssl_context_load_rejects_oob_length(int field) } TEST_ASSERT(len_p != NULL); -#if defined(MBEDTLS_SSL_ALPN) - if (field == TEST_SSL_CONTEXT_LOAD_OOB_ALPN) { - /* Double-check that the corrupted length overruns the serialized data - * by exactly one byte, as the ALPN field is the last one saved. */ - TEST_EQUAL((size_t) corrupt_len, - (size_t) (context_buf + context_buf_len - (len_p + 1)) + 1); + /* The ALPN field is the last one saved, so the buffer already ends right + * after it. The in_cid field is not last, so the loaded data is truncated + * right after the in_cid bytes to bring the end of the buffer within + * reach of the corrupted length. */ + if (field == TEST_SSL_CONTEXT_LOAD_OOB_CID) { + load_len = (size_t) (len_p + 1 - context_buf) + sentinel_len; + } else { + load_len = context_buf_len; } -#endif + + /* Double-check that the corrupted length overruns the loaded data by + * exactly one byte. */ + TEST_EQUAL((size_t) corrupt_len, + load_len - (size_t) (len_p + 1 - context_buf) + 1); *len_p = corrupt_len; + /* Load from a tightly allocated copy, so that reading even one byte past + * the end of the data overruns the allocation and gets caught by ASan or + * Valgrind. */ + TEST_CALLOC(load_buf, load_len); + memcpy(load_buf, context_buf, load_len); + /* Reinitialise the server SSL context so we can load into a fresh one. */ mbedtls_ssl_free(&server_ep.ssl); mbedtls_ssl_init(&server_ep.ssl); TEST_EQUAL(mbedtls_ssl_setup(&server_ep.ssl, &server_ep.conf), 0); - /* Note that this rejection alone does not prove the bounds check works: - * without it the ALPN case usually still fails with + /* Note that this rejection alone does not prove the bounds checks work: + * without them the load usually still fails with * MBEDTLS_ERR_SSL_BAD_INPUT_DATA, because the out-of-bounds bytes are - * unlikely to match a configured protocol so the alpn_chosen lookup - * fails afterwards. What the missing check leaks is the out-of-bounds - * read in memcmp() itself, so this test case only catches a regression - * when the test suite runs under ASan or Valgrind. */ - TEST_EQUAL(mbedtls_ssl_context_load(&server_ep.ssl, context_buf, - context_buf_len), + * unlikely to match a configured ALPN protocol, and the fields parsed + * after the overrun in_cid fail their own checks. What the missing + * checks leak is the out-of-bounds read itself, so this test case only + * catches a regression when the test suite runs under ASan or + * Valgrind. */ + TEST_EQUAL(mbedtls_ssl_context_load(&server_ep.ssl, load_buf, load_len), MBEDTLS_ERR_SSL_BAD_INPUT_DATA); exit: @@ -3161,6 +3180,7 @@ exit: mbedtls_test_ssl_endpoint_free(&server_ep); mbedtls_test_free_handshake_options(&options); mbedtls_free(context_buf); + mbedtls_free(load_buf); MD_OR_USE_PSA_DONE(); } /* END_CASE */ From 7e126796aa5320bc53a06fe351b1d47b21107758 Mon Sep 17 00:00:00 2001 From: Naveed Date: Sun, 26 Jul 2026 17:40:51 +0530 Subject: [PATCH 5/6] Keep the out_cid length byte in the CID case's truncated data With the loaded data cut right after the in_cid bytes, even the pristine in_cid length tripped the (end - p) < in_cid_len + 1u bounds check, since the out_cid length byte it accounts for was missing: the truncation, not the corruption, was what got rejected. Keep the out_cid length byte in the loaded data, so the pristine length passes the check and only the corrupted length trips it. Also document why this pre-existing check is the only one in the in_cid path that a regression test can guard: the in_cid_len > sizeof(in_cid) cap protects the memcpy() destination, and without it the overflow lands in the out_cid field inside the transform structure, where sanitizers cannot see it. Signed-off-by: Naveed --- tests/suites/test_suite_ssl.function | 31 ++++++++++++++++------------ 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 185d0404f4..f963dd247e 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -3066,16 +3066,21 @@ void ssl_context_load_rejects_oob_length(int field) server_cid, sizeof(server_cid)), 0); sentinel = server_cid; sentinel_len = sizeof(server_cid); - /* The in_cid field sits in the middle of the serialized data and - * its length is capped at MBEDTLS_SSL_CID_IN_LEN_MAX by a - * separate check, which is less than the size of the fields that - * follow it, so no corrupted length can reach past the end of - * the full buffer. Instead the loaded data is truncated right - * after the in_cid bytes (see the computation of load_len below) - * and the length is corrupted to reach one byte past that end. - * Staying under the cap ensures the bounds check, not the cap, - * is what must reject the corrupted length. */ - corrupt_len = (unsigned char) (sizeof(server_cid) + 1); + /* Corrupting the in_cid length can only exercise the + * (end - p) < in_cid_len + 1u bounds check in + * ssl_context_load(). The in_cid_len > sizeof(in_cid) cap that + * guards the memcpy() destination cannot be regression-tested: + * without it the overflowing bytes land in the adjacent out_cid + * field of the transform structure, out of sight of ASan, while + * the source read stays inside the buffer, since more than + * sizeof(in_cid) bytes are always serialized after in_cid. So + * the loaded data is truncated right after the out_cid length + * byte (see the computation of load_len below) and the in_cid + * length is corrupted to reach one byte past that end. Keeping + * the out_cid length byte in the loaded data ensures the + * pristine in_cid length still passes the bounds check: the + * corrupted length, not the truncation, is what it rejects. */ + corrupt_len = (unsigned char) (sizeof(server_cid) + 2); break; #endif #if defined(MBEDTLS_SSL_ALPN) @@ -3138,10 +3143,10 @@ void ssl_context_load_rejects_oob_length(int field) /* The ALPN field is the last one saved, so the buffer already ends right * after it. The in_cid field is not last, so the loaded data is truncated - * right after the in_cid bytes to bring the end of the buffer within - * reach of the corrupted length. */ + * right after the out_cid length byte that follows the in_cid bytes, to + * bring the end of the buffer within reach of the corrupted length. */ if (field == TEST_SSL_CONTEXT_LOAD_OOB_CID) { - load_len = (size_t) (len_p + 1 - context_buf) + sentinel_len; + load_len = (size_t) (len_p + 1 - context_buf) + sentinel_len + 1; } else { load_len = context_buf_len; } From 8acc73169850f6ca3bd6fdb86bd2e901006f3fdd Mon Sep 17 00:00:00 2001 From: Naveed Date: Sun, 26 Jul 2026 17:49:42 +0530 Subject: [PATCH 6/6] Guard the in_cid size check in the CID case The previous approach documented the in_cid_len > sizeof(in_cid) cap as untestable, since the overflowing memcpy() lands inside the transform structure where sanitizers cannot see it. It can be guarded without a sanitizer: grow the in_cid field in the loaded data (keep its bytes, pad to MBEDTLS_SSL_CID_IN_LEN_MAX + 1) and append the rest of the serialized data untouched, so the in-buffer bounds check passes and the cap is what must reject the length. Without the cap the remaining data parses correctly aligned and the load succeeds, so the test catches a regression by the return value alone. Signed-off-by: Naveed --- tests/suites/test_suite_ssl.function | 88 +++++++++++++++------------- 1 file changed, 47 insertions(+), 41 deletions(-) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index f963dd247e..52a478be8d 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -3026,7 +3026,7 @@ void ssl_context_load_rejects_oob_length(int field) mbedtls_test_ssl_endpoint client_ep, server_ep; unsigned char *context_buf = NULL; unsigned char *load_buf = NULL; - size_t context_buf_len, load_len, i; + size_t context_buf_len, load_len, field_off, i; const unsigned char *sentinel = NULL; size_t sentinel_len = 0; unsigned char corrupt_len = 0; @@ -3066,21 +3066,13 @@ void ssl_context_load_rejects_oob_length(int field) server_cid, sizeof(server_cid)), 0); sentinel = server_cid; sentinel_len = sizeof(server_cid); - /* Corrupting the in_cid length can only exercise the - * (end - p) < in_cid_len + 1u bounds check in - * ssl_context_load(). The in_cid_len > sizeof(in_cid) cap that - * guards the memcpy() destination cannot be regression-tested: - * without it the overflowing bytes land in the adjacent out_cid - * field of the transform structure, out of sight of ASan, while - * the source read stays inside the buffer, since more than - * sizeof(in_cid) bytes are always serialized after in_cid. So - * the loaded data is truncated right after the out_cid length - * byte (see the computation of load_len below) and the in_cid - * length is corrupted to reach one byte past that end. Keeping - * the out_cid length byte in the loaded data ensures the - * pristine in_cid length still passes the bounds check: the - * corrupted length, not the truncation, is what it rejects. */ - corrupt_len = (unsigned char) (sizeof(server_cid) + 2); + /* The smallest length that overflows the in_cid array in the SSL + * transform, so the size check on that array, not the in-buffer + * bounds check before it, is what must reject the corrupted + * length. The loaded data gets the in_cid field grown to this + * many bytes (see the construction of load_buf below) so that + * the in-buffer bounds check indeed passes. */ + corrupt_len = (unsigned char) (MBEDTLS_SSL_CID_IN_LEN_MAX + 1); break; #endif #if defined(MBEDTLS_SSL_ALPN) @@ -3141,42 +3133,56 @@ void ssl_context_load_rejects_oob_length(int field) } TEST_ASSERT(len_p != NULL); - /* The ALPN field is the last one saved, so the buffer already ends right - * after it. The in_cid field is not last, so the loaded data is truncated - * right after the out_cid length byte that follows the in_cid bytes, to - * bring the end of the buffer within reach of the corrupted length. */ + field_off = (size_t) (len_p + 1 - context_buf); + if (field == TEST_SSL_CONTEXT_LOAD_OOB_CID) { - load_len = (size_t) (len_p + 1 - context_buf) + sentinel_len + 1; + /* The in_cid field sits in the middle of the serialized data, so a + * corrupted length cannot reach past the end of the buffer. Instead + * grow the field itself: keep its bytes, pad it to corrupt_len bytes + * and append the untouched rest of the serialized data, so that the + * in-buffer bounds check passes and the size check on the in_cid + * array is reached. */ + load_len = context_buf_len + corrupt_len - sentinel_len; + TEST_CALLOC(load_buf, load_len); + memcpy(load_buf, context_buf, field_off + sentinel_len); + memset(load_buf + field_off + sentinel_len, 0xA5, + (size_t) corrupt_len - sentinel_len); + memcpy(load_buf + field_off + corrupt_len, + context_buf + field_off + sentinel_len, + context_buf_len - field_off - sentinel_len); + /* Double-check that corrupt_len bytes of field data plus the next + * length byte are present, which is what the in-buffer bounds check + * requires. */ + TEST_ASSERT(load_len - field_off >= (size_t) corrupt_len + 1); } else { + /* The ALPN field is the last one saved, so the buffer already ends + * right after it and the corrupted length reaches exactly one byte + * past the end. Load from a tightly allocated copy, so that reading + * even one byte past the end of the data overruns the allocation and + * gets caught by ASan or Valgrind. */ + TEST_EQUAL((size_t) corrupt_len, context_buf_len - field_off + 1); load_len = context_buf_len; + TEST_CALLOC(load_buf, load_len); + memcpy(load_buf, context_buf, load_len); } - /* Double-check that the corrupted length overruns the loaded data by - * exactly one byte. */ - TEST_EQUAL((size_t) corrupt_len, - load_len - (size_t) (len_p + 1 - context_buf) + 1); - - *len_p = corrupt_len; - - /* Load from a tightly allocated copy, so that reading even one byte past - * the end of the data overruns the allocation and gets caught by ASan or - * Valgrind. */ - TEST_CALLOC(load_buf, load_len); - memcpy(load_buf, context_buf, load_len); + load_buf[field_off - 1] = corrupt_len; /* Reinitialise the server SSL context so we can load into a fresh one. */ mbedtls_ssl_free(&server_ep.ssl); mbedtls_ssl_init(&server_ep.ssl); TEST_EQUAL(mbedtls_ssl_setup(&server_ep.ssl, &server_ep.conf), 0); - /* Note that this rejection alone does not prove the bounds checks work: - * without them the load usually still fails with - * MBEDTLS_ERR_SSL_BAD_INPUT_DATA, because the out-of-bounds bytes are - * unlikely to match a configured ALPN protocol, and the fields parsed - * after the overrun in_cid fail their own checks. What the missing - * checks leak is the out-of-bounds read itself, so this test case only - * catches a regression when the test suite runs under ASan or - * Valgrind. */ + /* How a regression is caught differs between the two cases. In the ALPN + * case, without the bounds check the load still fails, because the + * out-of-bounds bytes are unlikely to match a configured protocol; what + * the missing check leaks is the out-of-bounds read in memcmp() itself, + * so that case catches a regression only when the test suite runs under + * ASan or Valgrind. In the CID case, without the size check the + * memcpy() overflows the in_cid array inside the SSL transform (which + * stays within the enclosing struct, invisible to ASan), the rest of + * the data then parses correctly aligned and the load succeeds, so the + * check below catches a regression on its own. */ TEST_EQUAL(mbedtls_ssl_context_load(&server_ep.ssl, load_buf, load_len), MBEDTLS_ERR_SSL_BAD_INPUT_DATA);