From 83ab43da0c9f67c5069605552b1332ca5fadecf1 Mon Sep 17 00:00:00 2001 From: Dmitry Belyavskiy Date: Wed, 27 Jul 2022 12:15:07 +0200 Subject: [PATCH] Check that IV length is not less than zero As EVP_CIPHER_CTX_get_iv_length indicates failure with -1, this error should be processed. Also the result of this function shouldn't be assigned to an unsigned variable. Reviewed-by: Paul Dale Reviewed-by: Tomas Mraz Reviewed-by: Hugo Landau (Merged from https://github.com/openssl/openssl/pull/18891) --- crypto/cms/cms_enc.c | 5 +++++ crypto/cms/cms_pwri.c | 4 ++++ ssl/ktls.c | 7 ++++++- ssl/record/rec_layer_d1.c | 4 ++++ ssl/record/rec_layer_s3.c | 4 ++++ ssl/record/ssl3_record_tls13.c | 7 ++++++- ssl/statem/statem_srvr.c | 4 ++++ ssl/t1_lib.c | 15 ++++++++++----- 8 files changed, 43 insertions(+), 7 deletions(-) diff --git a/crypto/cms/cms_enc.c b/crypto/cms/cms_enc.c index a896148dd8..150b9ee4e1 100644 --- a/crypto/cms/cms_enc.c +++ b/crypto/cms/cms_enc.c @@ -83,6 +83,11 @@ BIO *ossl_cms_EncryptedContent_init_bio(CMS_EncryptedContentInfo *ec, calg->algorithm = OBJ_nid2obj(EVP_CIPHER_CTX_get_type(ctx)); /* Generate a random IV if we need one */ ivlen = EVP_CIPHER_CTX_get_iv_length(ctx); + if (ivlen < 0) { + ERR_raise(ERR_LIB_CMS, ERR_R_EVP_LIB); + goto err; + } + if (ivlen > 0) { if (RAND_bytes_ex(libctx, iv, ivlen, 0) <= 0) goto err; diff --git a/crypto/cms/cms_pwri.c b/crypto/cms/cms_pwri.c index 380240561f..1f73cb1008 100644 --- a/crypto/cms/cms_pwri.c +++ b/crypto/cms/cms_pwri.c @@ -96,6 +96,10 @@ CMS_RecipientInfo *CMS_add0_recipient_password(CMS_ContentInfo *cms, } ivlen = EVP_CIPHER_CTX_get_iv_length(ctx); + if (ivlen < 0) { + ERR_raise(ERR_LIB_CMS, ERR_R_EVP_LIB); + goto err; + } if (ivlen > 0) { if (RAND_bytes_ex(ossl_cms_ctx_get0_libctx(cms_ctx), iv, ivlen, 0) <= 0) diff --git a/ssl/ktls.c b/ssl/ktls.c index 2cd2a5bdd5..67499c3507 100644 --- a/ssl/ktls.c +++ b/ssl/ktls.c @@ -132,8 +132,11 @@ int ktls_configure_crypto(SSL_CONNECTION *s, const EVP_CIPHER *c, case SSL_AES128GCM: case SSL_AES256GCM: crypto_info->cipher_algorithm = CRYPTO_AES_NIST_GCM_16; - if (s->version == TLS1_3_VERSION) + if (s->version == TLS1_3_VERSION) { crypto_info->iv_len = EVP_CIPHER_CTX_get_iv_length(dd); + if (crypto_info->iv_len < 0) + return 0; + } else crypto_info->iv_len = EVP_GCM_TLS_FIXED_IV_LEN; break; @@ -141,6 +144,8 @@ int ktls_configure_crypto(SSL_CONNECTION *s, const EVP_CIPHER *c, case SSL_CHACHA20POLY1305: crypto_info->cipher_algorithm = CRYPTO_CHACHA20_POLY1305; crypto_info->iv_len = EVP_CIPHER_CTX_get_iv_length(dd); + if (crypto_info->iv_len < 0) + return 0; break; # endif case SSL_AES128: diff --git a/ssl/record/rec_layer_d1.c b/ssl/record/rec_layer_d1.c index 0170319866..9f90bee9d9 100644 --- a/ssl/record/rec_layer_d1.c +++ b/ssl/record/rec_layer_d1.c @@ -883,6 +883,10 @@ int do_dtls1_write(SSL_CONNECTION *sc, int type, const unsigned char *buf, int mode = EVP_CIPHER_CTX_get_mode(sc->enc_write_ctx); if (mode == EVP_CIPH_CBC_MODE) { eivlen = EVP_CIPHER_CTX_get_iv_length(sc->enc_write_ctx); + if (eivlen < 0) { + SSLfatal(sc, SSL_AD_INTERNAL_ERROR, SSL_R_LIBRARY_BUG); + return -1; + } if (eivlen <= 1) eivlen = 0; } diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c index f866792cdc..af539a0ea1 100644 --- a/ssl/record/rec_layer_s3.c +++ b/ssl/record/rec_layer_s3.c @@ -874,6 +874,10 @@ int do_ssl3_write(SSL_CONNECTION *s, int type, const unsigned char *buf, int mode = EVP_CIPHER_CTX_get_mode(s->enc_write_ctx); if (mode == EVP_CIPH_CBC_MODE) { eivlen = EVP_CIPHER_CTX_get_iv_length(s->enc_write_ctx); + if (eivlen < 0) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_LIBRARY_BUG); + return -1; + } if (eivlen <= 1) eivlen = 0; } else if (mode == EVP_CIPH_GCM_MODE) { diff --git a/ssl/record/ssl3_record_tls13.c b/ssl/record/ssl3_record_tls13.c index 10551c5afc..3bbc46b2af 100644 --- a/ssl/record/ssl3_record_tls13.c +++ b/ssl/record/ssl3_record_tls13.c @@ -25,7 +25,8 @@ int tls13_enc(SSL_CONNECTION *s, SSL3_RECORD *recs, size_t n_recs, int sending, { EVP_CIPHER_CTX *ctx; unsigned char iv[EVP_MAX_IV_LENGTH], recheader[SSL3_RT_HEADER_LENGTH]; - size_t ivlen, taglen, offset, loop, hdrlen; + size_t taglen, offset, loop, hdrlen; + int ivlen; unsigned char *staticiv; unsigned char *seq; int lenu, lenf; @@ -62,6 +63,10 @@ int tls13_enc(SSL_CONNECTION *s, SSL3_RECORD *recs, size_t n_recs, int sending, } ivlen = EVP_CIPHER_CTX_get_iv_length(ctx); + if (ivlen < 0) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); + return 0; + } if (s->early_data_state == SSL_EARLY_DATA_WRITING || s->early_data_state == SSL_EARLY_DATA_WRITE_RETRY) { diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index 87813633e8..db6d40682c 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -3809,6 +3809,10 @@ static int construct_stateless_ticket(SSL_CONNECTION *s, WPACKET *pkt, goto err; } iv_len = EVP_CIPHER_CTX_get_iv_length(ctx); + if (iv_len < 0) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); + goto err; + } } else { EVP_CIPHER *cipher = EVP_CIPHER_fetch(sctx->libctx, "AES-256-CBC", sctx->propq); diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 42fe13f12a..cb1e4055ec 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -1875,7 +1875,7 @@ SSL_TICKET_STATUS tls_decrypt_ticket(SSL_CONNECTION *s, SSL_SESSION *sess = NULL; unsigned char *sdec; const unsigned char *p; - int slen, renew_ticket = 0, declen; + int slen, ivlen, renew_ticket = 0, declen; SSL_TICKET_STATUS ret = SSL_TICKET_FATAL_ERR_OTHER; size_t mlen; unsigned char tick_hmac[EVP_MAX_MD_SIZE]; @@ -1989,9 +1989,14 @@ SSL_TICKET_STATUS tls_decrypt_ticket(SSL_CONNECTION *s, goto end; } + ivlen = EVP_CIPHER_CTX_get_iv_length(ctx); + if (ivlen < 0) { + ret = SSL_TICKET_FATAL_ERR_OTHER; + goto end; + } + /* Sanity check ticket length: must exceed keyname + IV + HMAC */ - if (eticklen <= - TLSEXT_KEYNAME_LENGTH + EVP_CIPHER_CTX_get_iv_length(ctx) + mlen) { + if (eticklen <= TLSEXT_KEYNAME_LENGTH + ivlen + mlen) { ret = SSL_TICKET_NO_DECRYPT; goto end; } @@ -2009,8 +2014,8 @@ SSL_TICKET_STATUS tls_decrypt_ticket(SSL_CONNECTION *s, } /* Attempt to decrypt session data */ /* Move p after IV to start of encrypted ticket, update length */ - p = etick + TLSEXT_KEYNAME_LENGTH + EVP_CIPHER_CTX_get_iv_length(ctx); - eticklen -= TLSEXT_KEYNAME_LENGTH + EVP_CIPHER_CTX_get_iv_length(ctx); + p = etick + TLSEXT_KEYNAME_LENGTH + ivlen; + eticklen -= TLSEXT_KEYNAME_LENGTH + ivlen; sdec = OPENSSL_malloc(eticklen); if (sdec == NULL || EVP_DecryptUpdate(ctx, sdec, &slen, p, (int)eticklen) <= 0) { -- 2.39.5