From 1d0c08b4963f5f7e1d1855e360417a11973d8455 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Fri, 9 Feb 2018 18:03:08 +0000 Subject: [PATCH] The function ssl_get_min_max_version() can fail We should always check the return code. This fixes a coverity issue. Reviewed-by: Tim Hudson (Merged from https://github.com/openssl/openssl/pull/5308) --- ssl/ssl_lib.c | 4 +++- ssl/ssl_locl.h | 5 +++-- ssl/statem/statem_clnt.c | 7 ++++++- ssl/t1_lib.c | 7 +++++-- 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index 68a9b19087..6a5c03de6a 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -2454,10 +2454,12 @@ STACK_OF(SSL_CIPHER) *SSL_get1_supported_ciphers(SSL *s) { STACK_OF(SSL_CIPHER) *sk = NULL, *ciphers; int i; + ciphers = SSL_get_ciphers(s); if (!ciphers) return NULL; - ssl_set_client_disabled(s); + if (!ssl_set_client_disabled(s)) + return NULL; for (i = 0; i < sk_SSL_CIPHER_num(ciphers); i++) { const SSL_CIPHER *c = sk_SSL_CIPHER_value(ciphers, i); if (!ssl_cipher_disabled(s, c, SSL_SECOP_CIPHER_SUPPORTED, 0)) { diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 221d5b903a..b590b53630 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -2310,7 +2310,8 @@ __owur int ssl_choose_server_version(SSL *s, CLIENTHELLO_MSG *hello, DOWNGRADE *dgrd); __owur int ssl_choose_client_version(SSL *s, int version, RAW_EXTENSION *extensions); -int ssl_get_min_max_version(const SSL *s, int *min_version, int *max_version); +__owur int ssl_get_min_max_version(const SSL *s, int *min_version, + int *max_version); __owur long tls1_default_timeout(void); __owur int dtls1_do_write(SSL *s, int type); @@ -2501,7 +2502,7 @@ __owur int tls1_set_peer_legacy_sigalg(SSL *s, const EVP_PKEY *pkey); __owur int tls1_lookup_md(const SIGALG_LOOKUP *lu, const EVP_MD **pmd); __owur size_t tls12_get_psigalgs(SSL *s, int sent, const uint16_t **psigs); __owur int tls12_check_peer_sigalg(SSL *s, uint16_t, EVP_PKEY *pkey); -void ssl_set_client_disabled(SSL *s); +__owur int ssl_set_client_disabled(SSL *s); __owur int ssl_cipher_disabled(SSL *s, const SSL_CIPHER *c, int op, int echde); __owur int ssl_handshake_hash(SSL *s, unsigned char *out, size_t outlen, diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index f224da6a64..d770706a6e 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -3650,8 +3650,13 @@ int ssl_cipher_list_to_bytes(SSL *s, STACK_OF(SSL_CIPHER) *sk, WPACKET *pkt) int i; size_t totlen = 0, len, maxlen, maxverok = 0; int empty_reneg_info_scsv = !s->renegotiate; + /* Set disabled masks for this session */ - ssl_set_client_disabled(s); + if (!ssl_set_client_disabled(s)) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_SSL_CIPHER_LIST_TO_BYTES, + SSL_R_NO_PROTOCOLS_AVAILABLE); + return 0; + } if (sk == NULL) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_SSL_CIPHER_LIST_TO_BYTES, diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 7109741a7d..3965be9d90 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -1051,12 +1051,14 @@ int SSL_get_peer_signature_type_nid(const SSL *s, int *pnid) * * Call ssl_cipher_disabled() to check that it's enabled or not. */ -void ssl_set_client_disabled(SSL *s) +int ssl_set_client_disabled(SSL *s) { s->s3->tmp.mask_a = 0; s->s3->tmp.mask_k = 0; ssl_set_sig_mask(&s->s3->tmp.mask_a, s, SSL_SECOP_SIGALG_MASK); - ssl_get_min_max_version(s, &s->s3->tmp.min_ver, &s->s3->tmp.max_ver); + if (ssl_get_min_max_version(s, &s->s3->tmp.min_ver, + &s->s3->tmp.max_ver) != 0) + return 0; #ifndef OPENSSL_NO_PSK /* with PSK there must be client callback set */ if (!s->psk_client_callback) { @@ -1070,6 +1072,7 @@ void ssl_set_client_disabled(SSL *s) s->s3->tmp.mask_k |= SSL_kSRP; } #endif + return 1; } /* -- 2.25.1