From 7e126796aa5320bc53a06fe351b1d47b21107758 Mon Sep 17 00:00:00 2001 From: Naveed Date: Sun, 26 Jul 2026 17:40:51 +0530 Subject: [PATCH] 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; }