coffee/mbedtls
1
0
Fork 0
mirror of https://github.com/Mbed-TLS/mbedtls.git synced 2026-10-28 18:07:34 +08:00
Code Issues Packages Projects Releases Wiki Activity

Fix the CID case of the OOB length regression test

Browse Source 
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 <naveed@bugqore.com>
This commit is contained in:
Naveed 2026-07-17 14:52:08 +05:30
parent e0177ae7e9
commit a3c9450cd2
1 changed files with 40 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

60
tests/suites/test_suite_ssl.function
View File

@ -3071,7 +3071,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;
@ -3111,10 +3112,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)
@ -3175,31 +3182,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:
@ -3207,6 +3226,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 */