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 <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5308)

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 68a9b190876a3a015d71dfc4456acc0cb8858d61..6a5c03de6af83e4450877a2df9bd66a89b85ffc8 100644 (file)
--- 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 221d5b903a5b80cbacf8183a8f3b95482b35e632..b590b53630ef56d990137221853dea9ebec3d6b1 100644 (file)
--- 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 f224da6a64844c9032e972f1cea99d04c75876ce..d770706a6ecd75c48b72554a468315980770e5ff 100644 (file)
--- 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 7109741a7dca1b4aa1682b330d77a9b7e667e218..3965be9d907b83a3d5920ae0c53edc4e20cfc065 100644 (file)
--- 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;
 }
 
 /*