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

Merge pull request #10929 from nvxbug/tls12-handshake-len-bounds-4.1

Browse Source 
Backport 4.1: Fix off-by-one over-read in TLS 1.2 ClientHello and CertificateRequest
This commit is contained in:
Gilles Peskine 2026-10-21 15:08:41 +00:00 committed by GitHub
commit 9ce9b40d84
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 59 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
ChangeLog.d/tls12-handshake-length-read-bounds.txt Normal file
View File

@ -0,0 +1,10 @@
Bugfix
* Fix a buffer over-read when parsing a TLS 1.2 ClientHello or
CertificateRequest that is truncated inside the two-byte length field
of the ciphersuite list (server side) or of the certificate
authorities list (client side): the length checks preceding those
fields reserved too few bytes, so the length field could be read past
the end of the message. Such messages are now rejected before the
read.
* The TLS 1.2 client now rejects a CertificateRequest message whose
supported_signature_algorithms list has an odd length.

31
library/ssl_tls12_client.c
View File

@ -2217,7 +2217,7 @@ static int ssl_parse_certificate_request(mbedtls_ssl_context *ssl)
buf = ssl->in_msg;
/* certificate_types */
if (ssl->in_hslen <= mbedtls_ssl_hs_hdr_len(ssl)) {
if (ssl->in_hslen < mbedtls_ssl_hs_hdr_len(ssl) + 1) {
MBEDTLS_SSL_DEBUG_MSG(1, ("bad certificate request message"));
mbedtls_ssl_send_alert_message(ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR);
@ -2227,16 +2227,10 @@ static int ssl_parse_certificate_request(mbedtls_ssl_context *ssl)
n = cert_type_len;
/*
* In the subsequent code there are two paths that read from buf:
* * the length of the signature algorithms field (if minor version of
* SSL is 3),
* * distinguished name length otherwise.
* Both reach at most the index:
* ...hdr_len + 2 + n,
* therefore the buffer length at this point must be greater than that
* regardless of the actual code path.
* The two-byte supported_signature_algorithms length is read next,
* at offset ...hdr_len + 1 + n.
*/
if (ssl->in_hslen <= mbedtls_ssl_hs_hdr_len(ssl) + 2 + n) {
if (ssl->in_hslen < mbedtls_ssl_hs_hdr_len(ssl) + 1 + n + 2) {
MBEDTLS_SSL_DEBUG_MSG(1, ("bad certificate request message"));
mbedtls_ssl_send_alert_message(ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR);
@ -2247,18 +2241,13 @@ static int ssl_parse_certificate_request(mbedtls_ssl_context *ssl)
sig_alg_len = MBEDTLS_GET_UINT16_BE(buf, mbedtls_ssl_hs_hdr_len(ssl) + 1 + n);
/*
* The furthest access in buf is in the loop few lines below:
* sig_alg[i + 1],
* where:
* sig_alg = buf + ...hdr_len + 3 + n,
* max(i) = sig_alg_len - 1.
* Therefore the furthest access is:
* buf[...hdr_len + 3 + n + sig_alg_len - 1 + 1],
* which reduces to:
* buf[...hdr_len + 3 + n + sig_alg_len],
* which is one less than we need the buf to be.
* The list is a sequence of two-byte values, so its length must be
* even. The message must have room for the list itself, plus the
* two-byte certificate_authorities length that follows it and is
* read below.
*/
if (ssl->in_hslen <= mbedtls_ssl_hs_hdr_len(ssl) + 3 + n + sig_alg_len) {
if (sig_alg_len % 2 != 0 ||
ssl->in_hslen < mbedtls_ssl_hs_hdr_len(ssl) + 1 + n + 2 + sig_alg_len + 2) {
MBEDTLS_SSL_DEBUG_MSG(1, ("bad certificate request message"));
mbedtls_ssl_send_alert_message(
ssl,

21
library/ssl_tls12_server.c
View File

@ -1016,7 +1016,7 @@ static int ssl_parse_client_hello(mbedtls_ssl_context *ssl)
sess_len = buf[34];
if (sess_len > sizeof(ssl->session_negotiate->id) ||
sess_len + 34 + 2 > msg_len) { /* 2 for cipherlist length field */
sess_len + 35 > msg_len) {
MBEDTLS_SSL_DEBUG_MSG(1, ("bad client hello message"));
mbedtls_ssl_send_alert_message(ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR);
@ -1037,9 +1037,16 @@ static int ssl_parse_client_hello(mbedtls_ssl_context *ssl)
#if defined(MBEDTLS_SSL_PROTO_DTLS)
if (ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM) {
cookie_offset = 35 + sess_len;
if (cookie_offset + 1 > msg_len) {
MBEDTLS_SSL_DEBUG_MSG(1, ("bad client hello message"));
mbedtls_ssl_send_alert_message(ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR);
return MBEDTLS_ERR_SSL_DECODE_ERROR;
}
cookie_len = buf[cookie_offset];
if (cookie_offset + 1 + cookie_len + 2 > msg_len) {
if (cookie_offset + 1 + cookie_len > msg_len) {
MBEDTLS_SSL_DEBUG_MSG(1, ("bad client hello message"));
mbedtls_ssl_send_alert_message(ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR);
@ -1085,6 +1092,16 @@ static int ssl_parse_client_hello(mbedtls_ssl_context *ssl)
#endif /* MBEDTLS_SSL_PROTO_DTLS */
ciph_offset = 35 + sess_len;
/* The two-byte ciphersuite list length field must be within the
* message: the checks above only guarantee that the fields before it
* (session id, and for DTLS the cookie) are. */
if (ciph_offset + 2 > msg_len) {
MBEDTLS_SSL_DEBUG_MSG(1, ("bad client hello message"));
mbedtls_ssl_send_alert_message(ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR);
return MBEDTLS_ERR_SSL_DECODE_ERROR;
}
ciph_len = MBEDTLS_GET_UINT16_BE(buf, ciph_offset);
if (ciph_len < 2 ||

20
tests/suites/test_suite_ssl.tls-defrag.data
View File

@ -63,6 +63,26 @@ Inject ClientHello - TLS 1.2 unknown ciphersuite (for reference)
depends_on:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED:PSA_WANT_KEY_TYPE_AES:PSA_WANT_ALG_SHA_256:PSA_WANT_ALG_GCM:PSA_WANT_ECC_SECP_R1_256:PSA_WANT_ECC_SECP_R1_384:PSA_WANT_ALG_SHA_1
inject_client_content_on_the_wire:MBEDTLS_PK_ECDSA:MBEDTLS_SSL_CLIENT_HELLO:"160303002f0100002b03030123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef000002cccc01000000":"got no ciphersuites in common":MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE
# See "ClientHello breakdown" above
# ClientHello body ending right after the session id: version (2) +
# random (32) + session id length (1) + session id (3) = 38 bytes, the
# minimum accepted, with the ciphersuite list length field missing
# entirely. Only run in TLS 1.2-only builds: when TLS 1.3 is enabled,
# the TLS 1.3 code parses the ClientHello first and rejects it itself.
Inject ClientHello - TLS 1.2 truncated after session id
depends_on:MBEDTLS_SSL_PROTO_TLS1_2:!MBEDTLS_SSL_PROTO_TLS1_3:MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED:PSA_WANT_KEY_TYPE_AES:PSA_WANT_ALG_SHA_256:PSA_WANT_ALG_GCM:PSA_WANT_ECC_SECP_R1_256:PSA_WANT_ECC_SECP_R1_384:PSA_WANT_ALG_SHA_1
inject_client_content_on_the_wire:MBEDTLS_PK_ECDSA:MBEDTLS_SSL_CLIENT_HELLO:"160303002a0100002603030123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef035f5f5f":"bad client hello message":MBEDTLS_ERR_SSL_DECODE_ERROR
# See "ClientHello breakdown" above
# ClientHello body ending inside the ciphersuite list length: version (2) +
# random (32) + session id length (1) + session id (2) + first length
# byte (1) = 38 bytes. Reading the full two-byte length field used to
# over-read one byte past the end of the message.
# Only run in TLS 1.2-only builds, see above.
Inject ClientHello - TLS 1.2 truncated inside ciphersuite list length
depends_on:MBEDTLS_SSL_PROTO_TLS1_2:!MBEDTLS_SSL_PROTO_TLS1_3:MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED:PSA_WANT_KEY_TYPE_AES:PSA_WANT_ALG_SHA_256:PSA_WANT_ALG_GCM:PSA_WANT_ECC_SECP_R1_256:PSA_WANT_ECC_SECP_R1_384:PSA_WANT_ALG_SHA_1
inject_client_content_on_the_wire:MBEDTLS_PK_ECDSA:MBEDTLS_SSL_CLIENT_HELLO:"160303002a0100002603030123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef025f5f00":"bad client hello message":MBEDTLS_ERR_SSL_DECODE_ERROR
# See "ClientHello breakdown" above
# ephemeral with secp256r1 + MBEDTLS_TLS1_3_AES_128_GCM_SHA256
Inject ClientHello - TLS 1.3 good (for reference)