Standardise our style for checking malloc failures
authorMatt Caswell <matt@openssl.org>
Fri, 30 Oct 2015 10:05:53 +0000 (10:05 +0000)
committerMatt Caswell <matt@openssl.org>
Mon, 9 Nov 2015 22:48:41 +0000 (22:48 +0000)
if we have a malloc |x = OPENSSL_malloc(...)| sometimes we check |x|
for NULL and sometimes we treat it as a boolean |if(!x) ...|. Standardise
the approach in libssl.

Reviewed-by: Kurt Roeckx <kurt@openssl.org>
13 files changed:
ssl/d1_lib.c
ssl/record/rec_layer_d1.c
ssl/record/rec_layer_s3.c
ssl/s3_lib.c
ssl/ssl_cert.c
ssl/ssl_ciph.c
ssl/ssl_conf.c
ssl/ssl_lib.c
ssl/ssl_sess.c
ssl/statem/statem_clnt.c
ssl/statem/statem_dtls.c
ssl/statem/statem_srvr.c
ssl/t1_lib.c

index b865ad4..6e70c56 100644 (file)
@@ -155,7 +155,7 @@ int dtls1_new(SSL *s)
     d1->link_mtu = 0;
     d1->mtu = 0;
 
-    if (!d1->buffered_messages || !d1->sent_messages) {
+    if (d1->buffered_messages == NULL || d1->sent_messages == NULL) {
         pqueue_free(d1->buffered_messages);
         pqueue_free(d1->sent_messages);
         OPENSSL_free(d1);
index 0133ae3..ebe486e 100644 (file)
@@ -137,8 +137,8 @@ int DTLS_RECORD_LAYER_new(RECORD_LAYER *rl)
     d->processed_rcds.q = pqueue_new();
     d->buffered_app_data.q = pqueue_new();
 
-    if (!d->unprocessed_rcds.q || !d->processed_rcds.q
-        || !d->buffered_app_data.q) {
+    if (d->unprocessed_rcds.q == NULL || d->processed_rcds.q == NULL
+        || d->buffered_app_data.q == NULL) {
         pqueue_free(d->unprocessed_rcds.q);
         pqueue_free(d->processed_rcds.q);
         pqueue_free(d->buffered_app_data.q);
index c9f1b71..ae31f5d 100644 (file)
@@ -530,7 +530,7 @@ int ssl3_write_bytes(SSL *s, int type, const void *buf_, int len)
                 packlen *= 4;
 
             wb->buf = OPENSSL_malloc(packlen);
-            if (!wb->buf) {
+            if (wb->buf == NULL) {
                 SSLerr(SSL_F_SSL3_WRITE_BYTES, ERR_R_MALLOC_FAILURE);
                 return -1;
             }
index 1c7e7a2..8b12761 100644 (file)
@@ -4311,7 +4311,7 @@ long ssl3_ctrl(SSL *s, int cmd, long larg, void *parg)
                 return 0;
 #endif
             ptmp = EVP_PKEY_new();
-            if (!ptmp)
+            if (ptmp == NULL)
                 return 0;
 #ifndef OPENSSL_NO_RSA
             else if (s->s3->peer_rsa_tmp)
@@ -4999,7 +4999,7 @@ static int ssl3_set_req_cert_type(CERT *c, const unsigned char *p, size_t len)
     if (len > 0xff)
         return 0;
     c->ctypes = OPENSSL_malloc(len);
-    if (!c->ctypes)
+    if (c->ctypes == NULL)
         return 0;
     memcpy(c->ctypes, p, len);
     c->ctype_num = len;
index 3304a1d..9a373b1 100644 (file)
@@ -282,7 +282,7 @@ CERT *ssl_cert_dup(CERT *cert)
     /* Configured sigalgs copied across */
     if (cert->conf_sigalgs) {
         ret->conf_sigalgs = OPENSSL_malloc(cert->conf_sigalgslen);
-        if (!ret->conf_sigalgs)
+        if (ret->conf_sigalgs == NULL)
             goto err;
         memcpy(ret->conf_sigalgs, cert->conf_sigalgs, cert->conf_sigalgslen);
         ret->conf_sigalgslen = cert->conf_sigalgslen;
@@ -291,7 +291,7 @@ CERT *ssl_cert_dup(CERT *cert)
 
     if (cert->client_sigalgs) {
         ret->client_sigalgs = OPENSSL_malloc(cert->client_sigalgslen);
-        if (!ret->client_sigalgs)
+        if (ret->client_sigalgs == NULL)
             goto err;
         memcpy(ret->client_sigalgs, cert->client_sigalgs,
                cert->client_sigalgslen);
@@ -303,7 +303,7 @@ CERT *ssl_cert_dup(CERT *cert)
     /* Copy any custom client certificate types */
     if (cert->ctypes) {
         ret->ctypes = OPENSSL_malloc(cert->ctype_num);
-        if (!ret->ctypes)
+        if (ret->ctypes == NULL)
             goto err;
         memcpy(ret->ctypes, cert->ctypes, cert->ctype_num);
         ret->ctype_num = cert->ctype_num;
@@ -968,7 +968,7 @@ int ssl_build_cert_chain(SSL *s, SSL_CTX *ctx, int flags)
     /* Rearranging and check the chain: add everything to a store */
     if (flags & SSL_BUILD_CHAIN_FLAG_CHECK) {
         chain_store = X509_STORE_new();
-        if (!chain_store)
+        if (chain_store == NULL)
             goto err;
         for (i = 0; i < sk_X509_num(cpk->chain); i++) {
             x = sk_X509_value(cpk->chain, i);
index 581c8a0..0cecd92 100644 (file)
@@ -1039,7 +1039,7 @@ static int ssl_cipher_strength_sort(CIPHER_ORDER **head_p,
     }
 
     number_uses = OPENSSL_zalloc(sizeof(int) * (max_strength_bits + 1));
-    if (!number_uses) {
+    if (number_uses == NULL) {
         SSLerr(SSL_F_SSL_CIPHER_STRENGTH_SORT, ERR_R_MALLOC_FAILURE);
         return (0);
     }
index 9c252fa..ad20f44 100644 (file)
@@ -487,12 +487,12 @@ static int cmd_DHParameters(SSL_CONF_CTX *cctx, const char *value)
     BIO *in = NULL;
     if (cctx->ctx || cctx->ssl) {
         in = BIO_new(BIO_s_file());
-        if (!in)
+        if (in == NULL)
             goto end;
         if (BIO_read_filename(in, value) <= 0)
             goto end;
         dh = PEM_read_bio_DHparams(in, NULL, NULL, NULL);
-        if (!dh)
+        if (dh == NULL)
             goto end;
     } else
         return 1;
index ec85256..b6e5127 100644 (file)
@@ -311,7 +311,7 @@ SSL *SSL_new(SSL_CTX *ctx)
     s->generate_session_id = ctx->generate_session_id;
 
     s->param = X509_VERIFY_PARAM_new();
-    if (!s->param)
+    if (s->param == NULL)
         goto err;
     X509_VERIFY_PARAM_inherit(s->param, ctx->param);
     s->quiet_shutdown = ctx->quiet_shutdown;
@@ -1547,7 +1547,7 @@ int SSL_CTX_set_alpn_protos(SSL_CTX *ctx, const unsigned char *protos,
 {
     OPENSSL_free(ctx->alpn_client_proto_list);
     ctx->alpn_client_proto_list = OPENSSL_malloc(protos_len);
-    if (!ctx->alpn_client_proto_list)
+    if (ctx->alpn_client_proto_list == NULL)
         return 1;
     memcpy(ctx->alpn_client_proto_list, protos, protos_len);
     ctx->alpn_client_proto_list_len = protos_len;
@@ -1565,7 +1565,7 @@ int SSL_set_alpn_protos(SSL *ssl, const unsigned char *protos,
 {
     OPENSSL_free(ssl->alpn_client_proto_list);
     ssl->alpn_client_proto_list = OPENSSL_malloc(protos_len);
-    if (!ssl->alpn_client_proto_list)
+    if (ssl->alpn_client_proto_list == NULL)
         return 1;
     memcpy(ssl->alpn_client_proto_list, protos, protos_len);
     ssl->alpn_client_proto_list_len = protos_len;
@@ -1708,7 +1708,7 @@ SSL_CTX *SSL_CTX_new(const SSL_METHOD *meth)
     }
 
     ret->param = X509_VERIFY_PARAM_new();
-    if (!ret->param)
+    if (ret->param == NULL)
         goto err;
 
     if ((ret->md5 = EVP_get_digestbyname("ssl3-md5")) == NULL) {
index 6f46b9f..9642746 100644 (file)
@@ -1009,7 +1009,7 @@ int SSL_set_session_ticket_ext(SSL *s, void *ext_data, int ext_len)
         s->tlsext_session_ticket = NULL;
         s->tlsext_session_ticket =
             OPENSSL_malloc(sizeof(TLS_SESSION_TICKET_EXT) + ext_len);
-        if (!s->tlsext_session_ticket) {
+        if (s->tlsext_session_ticket == NULL) {
             SSLerr(SSL_F_SSL_SET_SESSION_TICKET_EXT, ERR_R_MALLOC_FAILURE);
             return 0;
         }
index 4684098..330cee1 100644 (file)
@@ -2213,7 +2213,7 @@ MSG_PROCESS_RETURN tls_process_new_session_ticket(SSL *s, PACKET *pkt)
     s->session->tlsext_ticklen = 0;
 
     s->session->tlsext_tick = OPENSSL_malloc(ticklen);
-    if (!s->session->tlsext_tick) {
+    if (s->session->tlsext_tick == NULL) {
         SSLerr(SSL_F_TLS_PROCESS_NEW_SESSION_TICKET, ERR_R_MALLOC_FAILURE);
         goto err;
     }
@@ -2267,7 +2267,7 @@ MSG_PROCESS_RETURN tls_process_cert_status(SSL *s, PACKET *pkt)
     }
     OPENSSL_free(s->tlsext_ocsp_resp);
     s->tlsext_ocsp_resp = OPENSSL_malloc(resplen);
-    if (!s->tlsext_ocsp_resp) {
+    if (s->tlsext_ocsp_resp == NULL) {
         al = SSL_AD_INTERNAL_ERROR;
         SSLerr(SSL_F_TLS_PROCESS_CERT_STATUS, ERR_R_MALLOC_FAILURE);
         goto f_err;
@@ -2451,7 +2451,7 @@ psk_err:
         RSA *rsa;
         pmslen = SSL_MAX_MASTER_KEY_LENGTH;
         pms = OPENSSL_malloc(pmslen);
-        if (!pms)
+        if (pms == NULL)
             goto memerr;
 
         if (s->session->peer == NULL) {
@@ -2553,7 +2553,7 @@ psk_err:
 
         pmslen = DH_size(dh_clnt);
         pms = OPENSSL_malloc(pmslen);
-        if (!pms)
+        if (pms == NULL)
             goto memerr;
 
         /*
@@ -2693,7 +2693,7 @@ psk_err:
         }
         pmslen = (field_size + 7) / 8;
         pms = OPENSSL_malloc(pmslen);
-        if (!pms)
+        if (pms == NULL)
             goto memerr;
         n = ECDH_compute_key(pms, pmslen, srvr_ecpoint, clnt_ecdh, NULL);
         if (n <= 0 || pmslen != (size_t)n) {
@@ -2758,7 +2758,7 @@ psk_err:
 
         pmslen = 32;
         pms = OPENSSL_malloc(pmslen);
-        if (!pms)
+        if (pms == NULL)
             goto memerr;
 
         /*
@@ -2773,6 +2773,11 @@ psk_err:
 
         pkey_ctx = EVP_PKEY_CTX_new(pub_key =
                                     X509_get_pubkey(peer_cert), NULL);
+        if (pkey_ctx == NULL) {
+            SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_KEY_EXCHANGE,
+                   ERR_R_MALLOC_FAILURE);
+            goto err;
+        }
         /*
          * If we have send a certificate, and certificate key
          *
@@ -2989,8 +2994,12 @@ int tls_construct_client_verify(SSL *s)
 
     p = ssl_handshake_start(s);
     pkey = s->cert->key->privatekey;
-/* Create context from key and test if sha1 is allowed as digest */
+    /* Create context from key and test if sha1 is allowed as digest */
     pctx = EVP_PKEY_CTX_new(pkey, NULL);
+    if (pctx == NULL) {
+        SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_VERIFY, ERR_R_MALLOC_FAILURE);
+        goto err;
+    }
     EVP_PKEY_sign_init(pctx);
     if (EVP_PKEY_CTX_set_signature_md(pctx, EVP_sha1()) > 0) {
         if (!SSL_USE_SIGALGS(s))
index 58a0959..aafd28f 100644 (file)
@@ -1075,7 +1075,7 @@ int dtls1_buffer_message(SSL *s, int is_ccs)
     OPENSSL_assert(s->init_off == 0);
 
     frag = dtls1_hm_fragment_new(s->init_num, 0);
-    if (!frag)
+    if (frag == NULL)
         return 0;
 
     memcpy(frag->fragment, s->init_buf->data, s->init_num);
index e54672a..c418787 100644 (file)
@@ -2807,6 +2807,11 @@ MSG_PROCESS_RETURN tls_process_client_key_exchange(SSL *s, PACKET *pkt)
             pk = s->cert->pkeys[SSL_PKEY_GOST01].privatekey;
 
         pkey_ctx = EVP_PKEY_CTX_new(pk, NULL);
+        if (pkey_ctx == NULL) {
+            al = SSL_AD_INTERNAL_ERROR;
+            SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, ERR_R_MALLOC_FAILURE);
+            goto f_err;
+        }
         EVP_PKEY_decrypt_init(pkey_ctx);
         /*
          * If client certificate is present and is of the same type, maybe
@@ -3140,6 +3145,11 @@ MSG_PROCESS_RETURN tls_process_cert_verify(SSL *s, PACKET *pkt)
         unsigned char signature[64];
         int idx;
         EVP_PKEY_CTX *pctx = EVP_PKEY_CTX_new(pkey, NULL);
+        if (pctx == NULL) {
+            al = SSL_AD_INTERNAL_ERROR;
+            SSLerr(SSL_F_TLS_PROCESS_CERT_VERIFY, ERR_R_MALLOC_FAILURE);
+            goto f_err;
+        }
         EVP_PKEY_verify_init(pctx);
         if (len != 64) {
             fprintf(stderr, "GOST signature length is %d", len);
@@ -3337,7 +3347,7 @@ int tls_construct_new_session_ticket(SSL *s)
         return 0;
     }
     senc = OPENSSL_malloc(slen_full);
-    if (!senc) {
+    if (senc == NULL) {
         ossl_statem_set_error(s);
         return 0;
     }
index b31eae1..9607c2e 100644 (file)
@@ -599,7 +599,7 @@ int tls1_set_curves(unsigned char **pext, size_t *pextlen,
      */
     unsigned long dup_list = 0;
     clist = OPENSSL_malloc(ncurves * 2);
-    if (!clist)
+    if (clist == NULL)
         return 0;
     for (i = 0, p = clist; i < ncurves; i++) {
         unsigned long idmask;
@@ -1327,7 +1327,7 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf,
                  s->tlsext_session_ticket->data) {
             ticklen = s->tlsext_session_ticket->length;
             s->session->tlsext_tick = OPENSSL_malloc(ticklen);
-            if (!s->session->tlsext_tick)
+            if (s->session->tlsext_tick == NULL)
                 return NULL;
             memcpy(s->session->tlsext_tick,
                    s->tlsext_session_ticket->data, ticklen);
@@ -1787,7 +1787,7 @@ static int tls1_alpn_handle_client_hello(SSL *s, PACKET *pkt, int *al)
     if (r == SSL_TLSEXT_ERR_OK) {
         OPENSSL_free(s->s3->alpn_selected);
         s->s3->alpn_selected = OPENSSL_malloc(selected_len);
-        if (!s->s3->alpn_selected) {
+        if (s->s3->alpn_selected == NULL) {
             *al = SSL_AD_INTERNAL_ERROR;
             return -1;
         }
@@ -2496,7 +2496,7 @@ static int ssl_scan_serverhello_tlsext(SSL *s, PACKET *pkt, int *al)
                 return 0;
             }
             s->next_proto_negotiated = OPENSSL_malloc(selected_len);
-            if (!s->next_proto_negotiated) {
+            if (s->next_proto_negotiated == NULL) {
                 *al = TLS1_AD_INTERNAL_ERROR;
                 return 0;
             }
@@ -2528,7 +2528,7 @@ static int ssl_scan_serverhello_tlsext(SSL *s, PACKET *pkt, int *al)
             }
             OPENSSL_free(s->s3->alpn_selected);
             s->s3->alpn_selected = OPENSSL_malloc(len);
-            if (!s->s3->alpn_selected) {
+            if (s->s3->alpn_selected == NULL) {
                 *al = TLS1_AD_INTERNAL_ERROR;
                 return 0;
             }
@@ -3104,7 +3104,7 @@ static int tls_decrypt_ticket(SSL *s, const unsigned char *etick,
     p = etick + 16 + EVP_CIPHER_CTX_iv_length(&ctx);
     eticklen -= 16 + EVP_CIPHER_CTX_iv_length(&ctx);
     sdec = OPENSSL_malloc(eticklen);
-    if (!sdec) {
+    if (sdec == NULL) {
         EVP_CIPHER_CTX_cleanup(&ctx);
         return -1;
     }
@@ -3430,7 +3430,7 @@ static int tls1_set_shared_sigalgs(SSL *s)
     nmatch = tls12_shared_sigalgs(s, NULL, pref, preflen, allow, allowlen);
     if (nmatch) {
         salgs = OPENSSL_malloc(nmatch * sizeof(TLS_SIGALGS));
-        if (!salgs)
+        if (salgs == NULL)
             return 0;
         nmatch = tls12_shared_sigalgs(s, salgs, pref, preflen, allow, allowlen);
     } else {
@@ -4179,16 +4179,16 @@ DH *ssl_get_auto_dh(SSL *s)
 
     if (dh_secbits >= 128) {
         DH *dhp = DH_new();
-        if (!dhp)
+        if (dhp == NULL)
             return NULL;
         dhp->g = BN_new();
-        if (dhp->g)
+        if (dhp->g != NULL)
             BN_set_word(dhp->g, 2);
         if (dh_secbits >= 192)
             dhp->p = get_rfc3526_prime_8192(NULL);
         else
             dhp->p = get_rfc3526_prime_3072(NULL);
-        if (!dhp->p || !dhp->g) {
+        if (dhp->p == NULL || dhp->g == NULL) {
             DH_free(dhp);
             return NULL;
         }