mirror of
https://github.com/Mbed-TLS/mbedtls.git
synced 2025-07-30 22:43:08 +03:00
mbedtls_ssl_decrypt_buf(): fix buffer overread with stream cipher
With stream ciphers, add a check that there's enough room to read a MAC in the record. Without this check, subtracting the MAC length from the data length resulted in an integer underflow, causing the MAC calculation to try reading (SIZE_MAX + 1 - maclen) bytes of input, which is a buffer overread. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
This commit is contained in:
3
ChangeLog.d/ssl_decrypt_buf-short_record.txt
Normal file
3
ChangeLog.d/ssl_decrypt_buf-short_record.txt
Normal file
@ -0,0 +1,3 @@
|
|||||||
|
Security
|
||||||
|
* Fix a buffer overread when parsing short TLS application data records in
|
||||||
|
null-cipher cipher suites. Credit to OSS-Fuzz.
|
@ -1546,8 +1546,16 @@ int mbedtls_ssl_decrypt_buf(mbedtls_ssl_context const *ssl,
|
|||||||
|
|
||||||
#if defined(MBEDTLS_SSL_SOME_SUITES_USE_STREAM)
|
#if defined(MBEDTLS_SSL_SOME_SUITES_USE_STREAM)
|
||||||
if (ssl_mode == MBEDTLS_SSL_MODE_STREAM) {
|
if (ssl_mode == MBEDTLS_SSL_MODE_STREAM) {
|
||||||
|
if (rec->data_len < transform->maclen) {
|
||||||
|
MBEDTLS_SSL_DEBUG_MSG(1,
|
||||||
|
("Record too short for MAC:"
|
||||||
|
" %" MBEDTLS_PRINTF_SIZET " < %" MBEDTLS_PRINTF_SIZET,
|
||||||
|
rec->data_len, transform->maclen));
|
||||||
|
return MBEDTLS_ERR_SSL_INVALID_MAC;
|
||||||
|
}
|
||||||
|
|
||||||
/* The only supported stream cipher is "NULL",
|
/* The only supported stream cipher is "NULL",
|
||||||
* so there's nothing to do here.*/
|
* so there's no encryption to do here.*/
|
||||||
} else
|
} else
|
||||||
#endif /* MBEDTLS_SSL_SOME_SUITES_USE_STREAM */
|
#endif /* MBEDTLS_SSL_SOME_SUITES_USE_STREAM */
|
||||||
#if defined(MBEDTLS_GCM_C) || \
|
#if defined(MBEDTLS_GCM_C) || \
|
||||||
@ -2010,7 +2018,7 @@ hmac_failed_etm_enabled:
|
|||||||
unsigned char mac_expect[MBEDTLS_SSL_MAC_ADD] = { 0 };
|
unsigned char mac_expect[MBEDTLS_SSL_MAC_ADD] = { 0 };
|
||||||
unsigned char mac_peer[MBEDTLS_SSL_MAC_ADD] = { 0 };
|
unsigned char mac_peer[MBEDTLS_SSL_MAC_ADD] = { 0 };
|
||||||
|
|
||||||
/* If the initial value of padlen was such that
|
/* For CBC+MAC, If the initial value of padlen was such that
|
||||||
* data_len < maclen + padlen + 1, then padlen
|
* data_len < maclen + padlen + 1, then padlen
|
||||||
* got reset to 1, and the initial check
|
* got reset to 1, and the initial check
|
||||||
* data_len >= minlen + maclen + 1
|
* data_len >= minlen + maclen + 1
|
||||||
@ -2022,6 +2030,9 @@ hmac_failed_etm_enabled:
|
|||||||
* subtracted either padlen + 1 (if the padding was correct)
|
* subtracted either padlen + 1 (if the padding was correct)
|
||||||
* or 0 (if the padding was incorrect) since then,
|
* or 0 (if the padding was incorrect) since then,
|
||||||
* hence data_len >= maclen in any case.
|
* hence data_len >= maclen in any case.
|
||||||
|
*
|
||||||
|
* For stream ciphers, we checked above that
|
||||||
|
* data_len >= maclen.
|
||||||
*/
|
*/
|
||||||
rec->data_len -= transform->maclen;
|
rec->data_len -= transform->maclen;
|
||||||
ssl_extract_add_data_from_record(add_data, &add_data_len, rec,
|
ssl_extract_add_data_from_record(add_data, &add_data_len, rec,
|
||||||
|
Reference in New Issue
Block a user