diff --git a/ChangeLog.d/ssl-cookie-write-bounds.txt b/ChangeLog.d/ssl-cookie-write-bounds.txt new file mode 100644 index 0000000000..ba9a1292d3 --- /dev/null +++ b/ChangeLog.d/ssl-cookie-write-bounds.txt @@ -0,0 +1,6 @@ +Bugfix + * Fix mbedtls_ssl_cookie_write() writing up to 20 bytes past the end of + the output buffer when less than 36 bytes (or 52 bytes when the cookie + uses SHA-384) were available. The library's own callers always leave + enough room, so only applications that call it directly with a smaller + buffer were affected. diff --git a/library/ssl_cookie.c b/library/ssl_cookie.c index 11811ee30f..7c38ef44ea 100644 --- a/library/ssl_cookie.c +++ b/library/ssl_cookie.c @@ -159,7 +159,7 @@ int mbedtls_ssl_cookie_write(void *p_ctx, goto exit; } - status = psa_mac_sign_finish(&operation, *p, COOKIE_MD_OUTLEN, + status = psa_mac_sign_finish(&operation, *p, COOKIE_HMAC_LEN, &sign_mac_length); if (status != PSA_SUCCESS) { ret = PSA_TO_MBEDTLS_ERR(status); diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data index 3415c80c90..bb38cab847 100644 --- a/tests/suites/test_suite_ssl.data +++ b/tests/suites/test_suite_ssl.data @@ -3403,6 +3403,12 @@ cookie_parsing:"16fefd0000000000000000002f010000de000000000000011efefd7b72727272 Cookie parsing: one byte overread cookie_parsing:"16fefd0000000000000000002F010000de000000000000011efefd7b7272727272727272727272727272727272727272727272727272727272727d0001":MBEDTLS_ERR_SSL_DECODE_ERROR +Cookie write: output buffer of exactly the cookie length +cookie_write_bounds:32:0 + +Cookie write: output buffer one byte too short +cookie_write_bounds:31:MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL + TLS 1.3 srv Certificate msg - wrong vector lengths tls13_server_certificate_msg_invalid_vector_len diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 7a9a03adcd..2a8aff17d6 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -2,6 +2,7 @@ #include #include #include +#include #include #if defined(MBEDTLS_PK_HAVE_PRIVATE_HEADER) #include @@ -4405,6 +4406,40 @@ exit: } /* END_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_SSL_COOKIE_C */ +void cookie_write_bounds(int out_len, int exp_ret) +{ + mbedtls_ssl_cookie_ctx ctx; + const unsigned char cli_id[] = { 0x7f, 0x00, 0x00, 0x01, 0x12, 0x34 }; + unsigned char *buf = NULL; + unsigned char *p; + + mbedtls_ssl_cookie_init(&ctx); + PSA_INIT(); + + /* Allocate exactly out_len bytes so that Asan catches any write past + * the end of the output buffer. */ + TEST_CALLOC(buf, out_len); + + TEST_EQUAL(mbedtls_ssl_cookie_setup(&ctx), 0); + + p = buf; + TEST_EQUAL(mbedtls_ssl_cookie_write(&ctx, &p, buf + out_len, + cli_id, sizeof(cli_id)), exp_ret); + + if (exp_ret == 0) { + TEST_EQUAL(p - buf, out_len); + TEST_EQUAL(mbedtls_ssl_cookie_check(&ctx, buf, p - buf, + cli_id, sizeof(cli_id)), 0); + } + +exit: + mbedtls_free(buf); + mbedtls_ssl_cookie_free(&ctx); + PSA_DONE(); +} +/* END_CASE */ + /* BEGIN_CASE depends_on:MBEDTLS_TIMING_C:MBEDTLS_HAVE_TIME */ void timing_final_delay_accessor() {