u_len may be unused.

[openssl.git] / ssl / s3_srvr.c

diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index 05dc439c2de25b4fc0be0b3c6be3c232f1d02a6c..3e5c57af5ed490c5918e33d50847e2153f27e889 100644 (file)
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -227,7 +227,7 @@ int ssl3_accept(SSL *s)
     /* init things to blank */
     s->in_handshake++;
     if (!SSL_in_init(s) || SSL_in_before(s)) {
-        if(!SSL_clear(s))
+        if (!SSL_clear(s))
             return -1;
     }
 
@@ -882,7 +882,7 @@ int ssl3_send_hello_request(SSL *s)
 {
 
     if (s->state == SSL3_ST_SW_HELLO_REQ_A) {
-        if(!ssl_set_handshake_header(s, SSL3_MT_HELLO_REQUEST, 0)) {
+        if (!ssl_set_handshake_header(s, SSL3_MT_HELLO_REQUEST, 0)) {
             SSLerr(SSL_F_SSL3_SEND_HELLO_REQUEST, ERR_R_INTERNAL_ERROR);
             return -1;
         }
@@ -931,6 +931,16 @@ int ssl3_get_client_hello(SSL *s)
     s->first_packet = 0;
     d = p = (unsigned char *)s->init_msg;
 
+    /*
+     * 2 bytes for client version, SSL3_RANDOM_SIZE bytes for random, 1 byte
+     * for session id length
+     */
+    if (n < 2 + SSL3_RANDOM_SIZE + 1) {
+        al = SSL_AD_DECODE_ERROR;
+        SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_LENGTH_TOO_SHORT);
+        goto f_err;
+    }
+
     /*
      * use version from inside client hello, not from record header (may
      * differ: see RFC 2246, Appendix E, second paragraph)
@@ -963,6 +973,12 @@ int ssl3_get_client_hello(SSL *s)
         unsigned int session_length, cookie_length;
 
         session_length = *(p + SSL3_RANDOM_SIZE);
+
+        if (p + SSL3_RANDOM_SIZE + session_length + 1 >= d + n) {
+            al = SSL_AD_DECODE_ERROR;
+            SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_LENGTH_TOO_SHORT);
+            goto f_err;
+        }
         cookie_length = *(p + SSL3_RANDOM_SIZE + session_length + 1);
 
         if (cookie_length == 0)
@@ -976,6 +992,12 @@ int ssl3_get_client_hello(SSL *s)
     /* get the session-id */
     j = *(p++);
 
+    if (p + j > d + n) {
+        al = SSL_AD_DECODE_ERROR;
+        SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_LENGTH_TOO_SHORT);
+        goto f_err;
+    }
+
     s->hit = 0;
     /*
      * Versions before 0.9.7 always allow clients to resume sessions in
@@ -1020,8 +1042,19 @@ int ssl3_get_client_hello(SSL *s)
 
     if (SSL_IS_DTLS(s)) {
         /* cookie stuff */
+        if (p + 1 > d + n) {
+            al = SSL_AD_DECODE_ERROR;
+            SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_LENGTH_TOO_SHORT);
+            goto f_err;
+        }
         cookie_len = *(p++);
 
+        if (p + cookie_len > d + n) {
+            al = SSL_AD_DECODE_ERROR;
+            SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_LENGTH_TOO_SHORT);
+            goto f_err;
+        }
+
         /*
          * The ClientHello may contain a cookie even if the
          * HelloVerify message has not been sent--make sure that it
@@ -1087,27 +1120,33 @@ int ssl3_get_client_hello(SSL *s)
         }
     }
 
+    if (p + 2 > d + n) {
+        al = SSL_AD_DECODE_ERROR;
+        SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_LENGTH_TOO_SHORT);
+        goto f_err;
+    }
     n2s(p, i);
-    if ((i == 0) && (j != 0)) {
-        /* we need a cipher if we are not resuming a session */
+
+    if (i == 0) {
         al = SSL_AD_ILLEGAL_PARAMETER;
         SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_NO_CIPHERS_SPECIFIED);
         goto f_err;
     }
-    if ((p + i) >= (d + n)) {
+
+    /* i bytes of cipher data + 1 byte for compression length later */
+    if ((p + i + 1) > (d + n)) {
         /* not enough data */
         al = SSL_AD_DECODE_ERROR;
         SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_LENGTH_MISMATCH);
         goto f_err;
     }
-    if ((i > 0) && (ssl_bytes_to_cipher_list(s, p, i, &(ciphers))
-                    == NULL)) {
+    if (ssl_bytes_to_cipher_list(s, p, i, &(ciphers)) == NULL) {
         goto err;
     }
     p += i;
 
     /* If it is a hit, check that the cipher is in the list */
-    if ((s->hit) && (i > 0)) {
+    if (s->hit) {
         j = 0;
         id = s->session->cipher->id;
 
@@ -1336,8 +1375,8 @@ int ssl3_get_client_hello(SSL *s)
             sk_SSL_CIPHER_free(s->session->ciphers);
         s->session->ciphers = ciphers;
         if (ciphers == NULL) {
-            al = SSL_AD_ILLEGAL_PARAMETER;
-            SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_NO_CIPHERS_PASSED);
+            al = SSL_AD_INTERNAL_ERROR;
+            SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, ERR_R_INTERNAL_ERROR);
             goto f_err;
         }
         ciphers = NULL;
@@ -1501,7 +1540,7 @@ int ssl3_send_server_hello(SSL *s)
 #endif
         /* do the header */
         l = (p - d);
-        if(!ssl_set_handshake_header(s, SSL3_MT_SERVER_HELLO, l)) {
+        if (!ssl_set_handshake_header(s, SSL3_MT_SERVER_HELLO, l)) {
             SSLerr(SSL_F_SSL3_SEND_SERVER_HELLO, ERR_R_INTERNAL_ERROR);
             return -1;
         }
@@ -1516,7 +1555,7 @@ int ssl3_send_server_done(SSL *s)
 {
 
     if (s->state == SSL3_ST_SW_SRVR_DONE_A) {
-        if(!ssl_set_handshake_header(s, SSL3_MT_SERVER_DONE, 0)) {
+        if (!ssl_set_handshake_header(s, SSL3_MT_SERVER_DONE, 0)) {
             SSLerr(SSL_F_SSL3_SEND_SERVER_DONE, ERR_R_INTERNAL_ERROR);
             return -1;
         }
@@ -1966,7 +2005,7 @@ int ssl3_send_server_key_exchange(SSL *s)
             }
         }
 
-        if(!ssl_set_handshake_header(s, SSL3_MT_SERVER_KEY_EXCHANGE, n)) {
+        if (!ssl_set_handshake_header(s, SSL3_MT_SERVER_KEY_EXCHANGE, n)) {
             al = SSL_AD_HANDSHAKE_FAILURE;
             SSLerr(SSL_F_SSL3_SEND_SERVER_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR);
             goto f_err;
@@ -1980,8 +2019,7 @@ int ssl3_send_server_key_exchange(SSL *s)
     ssl3_send_alert(s, SSL3_AL_FATAL, al);
  err:
 #ifndef OPENSSL_NO_EC
-    if (encodedPoint != NULL)
-        OPENSSL_free(encodedPoint);
+    OPENSSL_free(encodedPoint);
     BN_CTX_free(bn_ctx);
 #endif
     EVP_MD_CTX_cleanup(&md_ctx);
@@ -2048,7 +2086,7 @@ int ssl3_send_certificate_request(SSL *s)
         p = ssl_handshake_start(s) + off;
         s2n(nl, p);
 
-        if(!ssl_set_handshake_header(s, SSL3_MT_CERTIFICATE_REQUEST, n)) {
+        if (!ssl_set_handshake_header(s, SSL3_MT_CERTIFICATE_REQUEST, n)) {
             SSLerr(SSL_F_SSL3_SEND_CERTIFICATE_REQUEST, ERR_R_INTERNAL_ERROR);
             return -1;
         }
@@ -2238,7 +2276,7 @@ int ssl3_get_client_key_exchange(SSL *s)
                                                         sizeof
                                                         (rand_premaster_secret));
         OPENSSL_cleanse(p, sizeof(rand_premaster_secret));
-        if(s->session->master_key_length < 0) {
+        if (s->session->master_key_length < 0) {
             al = SSL_AD_INTERNAL_ERROR;
             SSLerr(SSL_F_SSL3_GET_CLIENT_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR);
             goto f_err;
@@ -2335,7 +2373,7 @@ int ssl3_get_client_key_exchange(SSL *s)
                                                         session->master_key,
                                                         p, i);
         OPENSSL_cleanse(p, i);
-        if(s->session->master_key_length < 0) {
+        if (s->session->master_key_length < 0) {
             al = SSL_AD_INTERNAL_ERROR;
             SSLerr(SSL_F_SSL3_GET_CLIENT_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR);
             goto f_err;
@@ -2505,7 +2543,7 @@ int ssl3_get_client_key_exchange(SSL *s)
                                                         s->
                                                         session->master_key,
                                                         pms, outl);
-        if(s->session->master_key_length < 0) {
+        if (s->session->master_key_length < 0) {
             al = SSL_INTERNAL_ERROR;
             SSLerr(SSL_F_SSL3_GET_CLIENT_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR);
             goto f_err;
@@ -2658,7 +2696,7 @@ int ssl3_get_client_key_exchange(SSL *s)
                                                         p, i);
 
         OPENSSL_cleanse(p, i);
-        if(s->session->master_key_length < 0) {
+        if (s->session->master_key_length < 0) {
             al = SSL_AD_INTERNAL_ERROR;
             SSLerr(SSL_F_SSL3_GET_CLIENT_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR);
             goto f_err;
@@ -2724,16 +2762,14 @@ int ssl3_get_client_key_exchange(SSL *s)
         t += psk_len;
         s2n(psk_len, t);
 
-        if (s->session->psk_identity != NULL)
-            OPENSSL_free(s->session->psk_identity);
+        OPENSSL_free(s->session->psk_identity);
         s->session->psk_identity = BUF_strdup((char *)p);
         if (s->session->psk_identity == NULL) {
             SSLerr(SSL_F_SSL3_GET_CLIENT_KEY_EXCHANGE, ERR_R_MALLOC_FAILURE);
             goto psk_err;
         }
 
-        if (s->session->psk_identity_hint != NULL)
-            OPENSSL_free(s->session->psk_identity_hint);
+        OPENSSL_free(s->session->psk_identity_hint);
         s->session->psk_identity_hint = BUF_strdup(s->ctx->psk_identity_hint);
         if (s->ctx->psk_identity_hint != NULL &&
             s->session->psk_identity_hint == NULL) {
@@ -2747,7 +2783,7 @@ int ssl3_get_client_key_exchange(SSL *s)
                                                         session->master_key,
                                                         psk_or_pre_ms,
                                                         pre_ms_len);
-        if(s->session->master_key_length < 0) {
+        if (s->session->master_key_length < 0) {
             al = SSL_AD_INTERNAL_ERROR;
             SSLerr(SSL_F_SSL3_GET_CLIENT_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR);
             goto psk_err;
@@ -2782,8 +2818,7 @@ int ssl3_get_client_key_exchange(SSL *s)
                    SSL_R_BAD_SRP_PARAMETERS);
             goto f_err;
         }
-        if (s->session->srp_username != NULL)
-            OPENSSL_free(s->session->srp_username);
+        OPENSSL_free(s->session->srp_username);
         s->session->srp_username = BUF_strdup(s->srp_ctx.login);
         if (s->session->srp_username == NULL) {
             SSLerr(SSL_F_SSL3_GET_CLIENT_KEY_EXCHANGE, ERR_R_MALLOC_FAILURE);
@@ -2853,7 +2888,7 @@ int ssl3_get_client_key_exchange(SSL *s)
                                                         s->
                                                         session->master_key,
                                                         premaster_secret, 32);
-        if(s->session->master_key_length < 0) {
+        if (s->session->master_key_length < 0) {
             al = SSL_AD_INTERNAL_ERROR;
             SSLerr(SSL_F_SSL3_GET_CLIENT_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR);
             goto f_err;
@@ -2869,8 +2904,7 @@ int ssl3_get_client_key_exchange(SSL *s)
         EVP_PKEY_CTX_free(pkey_ctx);
         if (ret)
             return ret;
-        else
-            goto err;
+        goto err;
     } else {
         al = SSL_AD_HANDSHAKE_FAILURE;
         SSLerr(SSL_F_SSL3_GET_CLIENT_KEY_EXCHANGE, SSL_R_UNKNOWN_CIPHER_TYPE);
@@ -2886,8 +2920,7 @@ int ssl3_get_client_key_exchange(SSL *s)
 #ifndef OPENSSL_NO_EC
     EVP_PKEY_free(clnt_pub_pkey);
     EC_POINT_free(clnt_ecpoint);
-    if (srvr_ecdh != NULL)
-        EC_KEY_free(srvr_ecdh);
+    EC_KEY_free(srvr_ecdh);
     BN_CTX_free(bn_ctx);
 #endif
     return (-1);
@@ -3234,8 +3267,7 @@ int ssl3_get_client_certificate(SSL *s)
         EVP_PKEY_free(pkey);
     }
 
-    if (s->session->peer != NULL) /* This should not be needed */
-        X509_free(s->session->peer);
+    X509_free(s->session->peer);
     s->session->peer = sk_X509_shift(sk);
     s->session->verify_result = s->verify_result;
 
@@ -3250,8 +3282,7 @@ int ssl3_get_client_certificate(SSL *s)
             goto err;
         }
     }
-    if (s->session->sess_cert->cert_chain != NULL)
-        sk_X509_pop_free(s->session->sess_cert->cert_chain, X509_free);
+    sk_X509_pop_free(s->session->sess_cert->cert_chain, X509_free);
     s->session->sess_cert->cert_chain = sk;
     /*
      * Inconsistency alert: cert_chain does *not* include the peer's own
@@ -3266,10 +3297,8 @@ int ssl3_get_client_certificate(SSL *s)
         ssl3_send_alert(s, SSL3_AL_FATAL, al);
     }
  err:
-    if (x != NULL)
-        X509_free(x);
-    if (sk != NULL)
-        sk_X509_pop_free(sk, X509_free);
+    X509_free(x);
+    sk_X509_pop_free(sk, X509_free);
     return (ret);
 }
 
@@ -3431,7 +3460,7 @@ int ssl3_send_newsession_ticket(SSL *s)
         /* Skip ticket lifetime hint */
         p = ssl_handshake_start(s) + 4;
         s2n(len - 6, p);
-        if(!ssl_set_handshake_header(s, SSL3_MT_NEWSESSION_TICKET, len))
+        if (!ssl_set_handshake_header(s, SSL3_MT_NEWSESSION_TICKET, len))
             goto err;
         s->state = SSL3_ST_SW_SESSION_TICKET_B;
         OPENSSL_free(senc);
@@ -3440,8 +3469,7 @@ int ssl3_send_newsession_ticket(SSL *s)
     /* SSL3_ST_SW_SESSION_TICKET_B */
     return ssl_do_write(s);
  err:
-    if (senc)
-        OPENSSL_free(senc);
+    OPENSSL_free(senc);
     EVP_CIPHER_CTX_cleanup(&ctx);
     HMAC_CTX_cleanup(&hctx);
     return -1;