Skip to content

Commit

Permalink
Standardise our style for checking malloc failures
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
@mattcaswell
mattcaswell committed Nov 9, 2015
1 parent 3457e7a commit a71edf3
Show file tree
Hide file tree
Showing 13 changed files with 56 additions and 37 deletions.
2 changes: 1 addition & 1 deletion ssl/d1_lib.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions ssl/record/rec_layer_d1.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion ssl/record/rec_layer_s3.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
4 changes: 2 additions & 2 deletions ssl/s3_lib.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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;
Expand Down
8 changes: 4 additions & 4 deletions ssl/ssl_cert.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion ssl/ssl_ciph.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
4 changes: 2 additions & 2 deletions ssl/ssl_conf.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 4 additions & 4 deletions ssl/ssl_lib.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion ssl/ssl_sess.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
23 changes: 16 additions & 7 deletions ssl/statem/statem_clnt.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -2451,7 +2451,7 @@ int tls_construct_client_key_exchange(SSL *s)
RSA *rsa;
pmslen = SSL_MAX_MASTER_KEY_LENGTH;
pms = OPENSSL_malloc(pmslen);
if (!pms)
if (pms == NULL)
goto memerr;

if (s->session->peer == NULL) {
Expand Down Expand Up @@ -2553,7 +2553,7 @@ int tls_construct_client_key_exchange(SSL *s)

pmslen = DH_size(dh_clnt);
pms = OPENSSL_malloc(pmslen);
if (!pms)
if (pms == NULL)
goto memerr;

/*
Expand Down Expand Up @@ -2693,7 +2693,7 @@ int tls_construct_client_key_exchange(SSL *s)
}
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) {
Expand Down Expand Up @@ -2758,7 +2758,7 @@ int tls_construct_client_key_exchange(SSL *s)

pmslen = 32;
pms = OPENSSL_malloc(pmslen);
if (!pms)
if (pms == NULL)
goto memerr;

/*
Expand All @@ -2773,6 +2773,11 @@ int tls_construct_client_key_exchange(SSL *s)

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
*
Expand Down Expand Up @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion ssl/statem/statem_dtls.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
12 changes: 11 additions & 1 deletion ssl/statem/statem_srvr.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
Expand Down
20 changes: 10 additions & 10 deletions ssl/t1_lib.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
}
Expand Down

0 comments on commit a71edf3

Please sign in to comment.