diff options
author | Matt Caswell <matt@openssl.org> | 2021-03-18 16:52:10 +0000 |
---|---|---|
committer | Matt Caswell <matt@openssl.org> | 2021-03-25 09:48:08 +0000 |
commit | 39a140597d874e554b736885ac4dea16ac40a87a (patch) | |
tree | 4111ade117e62d5eb609109e6c47d7a866660046 /ssl/statem | |
parent | 02b1636fe3db274497304a3e95a4e32ced7e841b (diff) |
Ensure buffer/length pairs are always in sync
Following on from CVE-2021-3449 which was caused by a non-zero length
associated with a NULL buffer, other buffer/length pairs are updated to
ensure that they too are always in sync.
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
Diffstat (limited to 'ssl/statem')
-rw-r--r-- | ssl/statem/extensions.c | 1 | ||||
-rw-r--r-- | ssl/statem/extensions_clnt.c | 13 | ||||
-rw-r--r-- | ssl/statem/statem_clnt.c | 7 | ||||
-rw-r--r-- | ssl/statem/statem_srvr.c | 15 |
4 files changed, 30 insertions, 6 deletions
diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c index bc41b36594..0ce436d082 100644 --- a/ssl/statem/extensions.c +++ b/ssl/statem/extensions.c @@ -1124,6 +1124,7 @@ static int init_sig_algs_cert(SSL *s, ossl_unused unsigned int context) /* Clear any signature algorithms extension received */ OPENSSL_free(s->s3.tmp.peer_cert_sigalgs); s->s3.tmp.peer_cert_sigalgs = NULL; + s->s3.tmp.peer_cert_sigalgslen = 0; return 1; } diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c index cac713fff0..b3ef1bc16a 100644 --- a/ssl/statem/extensions_clnt.c +++ b/ssl/statem/extensions_clnt.c @@ -810,6 +810,7 @@ EXT_RETURN tls_construct_ctos_early_data(SSL *s, WPACKET *pkt, OPENSSL_free(s->psksession_id); s->psksession_id = OPENSSL_memdup(id, idlen); if (s->psksession_id == NULL) { + s->psksession_id_len = 0; SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); return EXT_RETURN_FAIL; } @@ -1337,6 +1338,7 @@ int tls_parse_stoc_ec_pt_formats(SSL *s, PACKET *pkt, unsigned int context, OPENSSL_free(s->ext.peer_ecpointformats); s->ext.peer_ecpointformats = OPENSSL_malloc(ecpointformats_len); if (s->ext.peer_ecpointformats == NULL) { + s->ext.peer_ecpointformats_len = 0; SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); return 0; } @@ -1446,8 +1448,12 @@ int tls_parse_stoc_sct(SSL *s, PACKET *pkt, unsigned int context, X509 *x, s->ext.scts_len = (uint16_t)size; if (size > 0) { s->ext.scts = OPENSSL_malloc(size); - if (s->ext.scts == NULL - || !PACKET_copy_bytes(pkt, s->ext.scts, size)) { + if (s->ext.scts == NULL) { + s->ext.scts_len = 0; + SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_MALLOC_FAILURE); + return 0; + } + if (!PACKET_copy_bytes(pkt, s->ext.scts, size)) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); return 0; } @@ -1541,6 +1547,7 @@ int tls_parse_stoc_npn(SSL *s, PACKET *pkt, unsigned int context, X509 *x, OPENSSL_free(s->ext.npn); s->ext.npn = OPENSSL_malloc(selected_len); if (s->ext.npn == NULL) { + s->ext.npn_len = 0; SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); return 0; } @@ -1578,6 +1585,7 @@ int tls_parse_stoc_alpn(SSL *s, PACKET *pkt, unsigned int context, X509 *x, OPENSSL_free(s->s3.alpn_selected); s->s3.alpn_selected = OPENSSL_malloc(len); if (s->s3.alpn_selected == NULL) { + s->s3.alpn_selected_len = 0; SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); return 0; } @@ -1606,6 +1614,7 @@ int tls_parse_stoc_alpn(SSL *s, PACKET *pkt, unsigned int context, X509 *x, s->session->ext.alpn_selected = OPENSSL_memdup(s->s3.alpn_selected, s->s3.alpn_selected_len); if (s->session->ext.alpn_selected == NULL) { + s->session->ext.alpn_selected_len = 0; SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); return 0; } diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index 666ee43363..8686b1e684 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -2352,6 +2352,7 @@ MSG_PROCESS_RETURN tls_process_certificate_request(SSL *s, PACKET *pkt) s->s3.tmp.ctype_len = 0; OPENSSL_free(s->pha_context); s->pha_context = NULL; + s->pha_context_len = 0; if (!PACKET_get_length_prefixed_1(pkt, &reqctx) || !PACKET_memdup(&reqctx, &s->pha_context, &s->pha_context_len)) { @@ -2642,14 +2643,15 @@ int tls_process_cert_status_body(SSL *s, PACKET *pkt) } s->ext.ocsp.resp = OPENSSL_malloc(resplen); if (s->ext.ocsp.resp == NULL) { + s->ext.ocsp.resp_len = 0; SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_MALLOC_FAILURE); return 0; } + s->ext.ocsp.resp_len = resplen; if (!PACKET_copy_bytes(pkt, s->ext.ocsp.resp, resplen)) { SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_LENGTH_MISMATCH); return 0; } - s->ext.ocsp.resp_len = resplen; return 1; } @@ -3313,9 +3315,11 @@ int tls_construct_client_key_exchange(SSL *s, WPACKET *pkt) err: OPENSSL_clear_free(s->s3.tmp.pms, s->s3.tmp.pmslen); s->s3.tmp.pms = NULL; + s->s3.tmp.pmslen = 0; #ifndef OPENSSL_NO_PSK OPENSSL_clear_free(s->s3.tmp.psk, s->s3.tmp.psklen); s->s3.tmp.psk = NULL; + s->s3.tmp.psklen = 0; #endif return 0; } @@ -3387,6 +3391,7 @@ int tls_client_key_exchange_post_work(SSL *s) err: OPENSSL_clear_free(pms, pmslen); s->s3.tmp.pms = NULL; + s->s3.tmp.pmslen = 0; return 0; } diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index aca17868b1..bad3619170 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -2116,6 +2116,7 @@ int tls_handle_alpn(SSL *s) OPENSSL_free(s->s3.alpn_selected); s->s3.alpn_selected = OPENSSL_memdup(selected, selected_len); if (s->s3.alpn_selected == NULL) { + s->s3.alpn_selected_len = 0; SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); return 0; } @@ -2725,10 +2726,15 @@ int tls_construct_certificate_request(SSL *s, WPACKET *pkt) if (s->post_handshake_auth == SSL_PHA_REQUEST_PENDING) { OPENSSL_free(s->pha_context); s->pha_context_len = 32; - if ((s->pha_context = OPENSSL_malloc(s->pha_context_len)) == NULL - || RAND_bytes_ex(s->ctx->libctx, s->pha_context, + if ((s->pha_context = OPENSSL_malloc(s->pha_context_len)) == NULL) { + s->pha_context_len = 0; + SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); + return 0; + } + if (RAND_bytes_ex(s->ctx->libctx, s->pha_context, s->pha_context_len) <= 0 - || !WPACKET_sub_memcpy_u8(pkt, s->pha_context, s->pha_context_len)) { + || !WPACKET_sub_memcpy_u8(pkt, s->pha_context, + s->pha_context_len)) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); return 0; } @@ -2828,6 +2834,7 @@ static int tls_process_cke_psk_preamble(SSL *s, PACKET *pkt) OPENSSL_cleanse(psk, psklen); if (s->s3.tmp.psk == NULL) { + s->s3.tmp.psklen = 0; SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_MALLOC_FAILURE); return 0; } @@ -3329,6 +3336,7 @@ MSG_PROCESS_RETURN tls_process_client_key_exchange(SSL *s, PACKET *pkt) #ifndef OPENSSL_NO_PSK OPENSSL_clear_free(s->s3.tmp.psk, s->s3.tmp.psklen); s->s3.tmp.psk = NULL; + s->s3.tmp.psklen = 0; #endif return MSG_PROCESS_ERROR; } @@ -3921,6 +3929,7 @@ int tls_construct_new_session_ticket(SSL *s, WPACKET *pkt) s->session->ext.alpn_selected = OPENSSL_memdup(s->s3.alpn_selected, s->s3.alpn_selected_len); if (s->session->ext.alpn_selected == NULL) { + s->session->ext.alpn_selected_len = 0; SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_MALLOC_FAILURE); goto err; } |