From 2c4a056f59a6819b8a0d40e3a7e11cf6d35b3e88 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Fri, 3 Jun 2016 11:59:19 +0100 Subject: [PATCH] Handle a memory allocation failure in ssl3_init_finished_mac() The ssl3_init_finished_mac() function can fail, in which case we need to propagate the error up through the stack. RT#3198 Reviewed-by: Rich Salz --- include/openssl/ssl.h | 1 + ssl/s3_enc.c | 11 +++++++++-- ssl/ssl_err.c | 1 + ssl/ssl_locl.h | 2 +- ssl/statem/statem.c | 8 ++++++-- ssl/statem/statem_clnt.c | 5 ++++- ssl/statem/statem_srvr.c | 11 ++++++++--- 7 files changed, 30 insertions(+), 9 deletions(-) diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index c6c357642a..2779fff03a 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -2047,6 +2047,7 @@ void ERR_load_SSL_strings(void); # define SSL_F_SSL3_GENERATE_KEY_BLOCK 238 # define SSL_F_SSL3_GENERATE_MASTER_SECRET 388 # define SSL_F_SSL3_GET_RECORD 143 +# define SSL_F_SSL3_INIT_FINISHED_MAC 339 # define SSL_F_SSL3_OUTPUT_CERT_CHAIN 147 # define SSL_F_SSL3_READ_BYTES 148 # define SSL_F_SSL3_READ_N 149 diff --git a/ssl/s3_enc.c b/ssl/s3_enc.c index cb571c1d07..f7089bd6fb 100644 --- a/ssl/s3_enc.c +++ b/ssl/s3_enc.c @@ -326,11 +326,18 @@ void ssl3_cleanup_key_block(SSL *s) s->s3->tmp.key_block_length = 0; } -void ssl3_init_finished_mac(SSL *s) +int ssl3_init_finished_mac(SSL *s) { + BIO *buf = BIO_new(BIO_s_mem()); + + if (buf == NULL) { + SSLerr(SSL_F_SSL3_INIT_FINISHED_MAC, ERR_R_MALLOC_FAILURE); + return 0; + } ssl3_free_digest_list(s); - s->s3->handshake_buffer = BIO_new(BIO_s_mem()); + s->s3->handshake_buffer = buf; (void)BIO_set_close(s->s3->handshake_buffer, BIO_CLOSE); + return 1; } /* diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c index 377061b47d..0c46768482 100644 --- a/ssl/ssl_err.c +++ b/ssl/ssl_err.c @@ -60,6 +60,7 @@ static ERR_STRING_DATA SSL_str_functs[] = { {ERR_FUNC(SSL_F_SSL3_GENERATE_MASTER_SECRET), "ssl3_generate_master_secret"}, {ERR_FUNC(SSL_F_SSL3_GET_RECORD), "ssl3_get_record"}, + {ERR_FUNC(SSL_F_SSL3_INIT_FINISHED_MAC), "ssl3_init_finished_mac"}, {ERR_FUNC(SSL_F_SSL3_OUTPUT_CERT_CHAIN), "ssl3_output_cert_chain"}, {ERR_FUNC(SSL_F_SSL3_READ_BYTES), "ssl3_read_bytes"}, {ERR_FUNC(SSL_F_SSL3_READ_N), "ssl3_read_n"}, diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 243535fe50..35fd3fc7ac 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -1859,7 +1859,7 @@ __owur EVP_PKEY *ssl_dh_to_pkey(DH *dh); __owur const SSL_CIPHER *ssl3_get_cipher_by_char(const unsigned char *p); __owur int ssl3_put_cipher_by_char(const SSL_CIPHER *c, unsigned char *p); -void ssl3_init_finished_mac(SSL *s); +int ssl3_init_finished_mac(SSL *s); __owur int ssl3_setup_key_block(SSL *s); __owur int ssl3_change_cipher_state(SSL *s, int which); void ssl3_cleanup_key_block(SSL *s); diff --git a/ssl/statem/statem.c b/ssl/statem/statem.c index 0b0595d237..28483e7944 100644 --- a/ssl/statem/statem.c +++ b/ssl/statem/statem.c @@ -332,8 +332,12 @@ static int state_machine(SSL *s, int server) goto end; } - if (!server || st->state != MSG_FLOW_RENEGOTIATE) - ssl3_init_finished_mac(s); + if (!server || st->state != MSG_FLOW_RENEGOTIATE) { + if (!ssl3_init_finished_mac(s)) { + ossl_statem_set_error(s); + goto end; + } + } if (server) { if (st->state != MSG_FLOW_RENEGOTIATE) { diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index ecbc43b3d0..a97f0cc6bf 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -391,7 +391,10 @@ WORK_STATE ossl_statem_client_pre_work(SSL *s, WORK_STATE wst) s->shutdown = 0; if (SSL_IS_DTLS(s)) { /* every DTLS ClientHello resets Finished MAC */ - ssl3_init_finished_mac(s); + if (!ssl3_init_finished_mac(s)) { + ossl_statem_set_error(s); + return WORK_ERROR; + } } break; diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index 67df5f2640..71dd27f7bc 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -496,15 +496,20 @@ WORK_STATE ossl_statem_server_post_work(SSL *s, WORK_STATE wst) case TLS_ST_SW_HELLO_REQ: if (statem_flush(s) != 1) return WORK_MORE_A; - ssl3_init_finished_mac(s); + if (!ssl3_init_finished_mac(s)) { + ossl_statem_set_error(s); + return WORK_ERROR; + } break; case DTLS_ST_SW_HELLO_VERIFY_REQUEST: if (statem_flush(s) != 1) return WORK_MORE_A; /* HelloVerifyRequest resets Finished MAC */ - if (s->version != DTLS1_BAD_VER) - ssl3_init_finished_mac(s); + if (s->version != DTLS1_BAD_VER && !ssl3_init_finished_mac(s)) { + ossl_statem_set_error(s); + return WORK_ERROR; + } /* * The next message should be another ClientHello which we need to * treat like it was the first packet -- 2.34.1