From a71edf3ba275b946224b5bcded0a8ecfce1855c0 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Fri, 30 Oct 2015 10:05:53 +0000 Subject: Standardise our style for checking malloc failures 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 --- ssl/d1_lib.c | 2 +- ssl/record/rec_layer_d1.c | 4 ++-- ssl/record/rec_layer_s3.c | 2 +- ssl/s3_lib.c | 4 ++-- ssl/ssl_cert.c | 8 ++++---- ssl/ssl_ciph.c | 2 +- ssl/ssl_conf.c | 4 ++-- ssl/ssl_lib.c | 8 ++++---- ssl/ssl_sess.c | 2 +- ssl/statem/statem_clnt.c | 23 ++++++++++++++++------- ssl/statem/statem_dtls.c | 2 +- ssl/statem/statem_srvr.c | 12 +++++++++++- ssl/t1_lib.c | 20 ++++++++++---------- 13 files changed, 56 insertions(+), 37 deletions(-) diff --git a/ssl/d1_lib.c b/ssl/d1_lib.c index b865ad42b2..6e70c56ce7 100644 --- a/ssl/d1_lib.c +++ b/ssl/d1_lib.c @@ -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); diff --git a/ssl/record/rec_layer_d1.c b/ssl/record/rec_layer_d1.c index 0133ae3f18..ebe486e419 100644 --- a/ssl/record/rec_layer_d1.c +++ b/ssl/record/rec_layer_d1.c @@ -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); diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c index c9f1b712c8..ae31f5dd66 100644 --- a/ssl/record/rec_layer_s3.c +++ b/ssl/record/rec_layer_s3.c @@ -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; } diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index 1c7e7a2e92..8b1276174b 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -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; diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c index 3304a1d164..9a373b19ab 100644 --- a/ssl/ssl_cert.c +++ b/ssl/ssl_cert.c @@ -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); diff --git a/ssl/ssl_ciph.c b/ssl/ssl_ciph.c index 581c8a06ae..0cecd929a3 100644 --- a/ssl/ssl_ciph.c +++ b/ssl/ssl_ciph.c @@ -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); } diff --git a/ssl/ssl_conf.c b/ssl/ssl_conf.c index 9c252fa609..ad20f4434c 100644 --- a/ssl/ssl_conf.c +++ b/ssl/ssl_conf.c @@ -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; diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index ec852569b1..b6e5127f0c 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -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) { diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c index 6f46b9f37e..964274600c 100644 --- a/ssl/ssl_sess.c +++ b/ssl/ssl_sess.c @@ -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; } diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index 4684098250..330cee13fc 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -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)) diff --git a/ssl/statem/statem_dtls.c b/ssl/statem/statem_dtls.c index 58a09594c9..aafd28f8de 100644 --- a/ssl/statem/statem_dtls.c +++ b/ssl/statem/statem_dtls.c @@ -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); diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index e54672a619..c4187876fd 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -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; } diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index b31eae180c..9607c2e02a 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -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; } -- cgit v1.2.3