From 918bb8652969fd53f0c390c1cd909265ed502c7e Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Wed, 4 Mar 2015 17:49:51 +0000 Subject: [PATCH] Unchecked malloc fixes Miscellaneous unchecked malloc fixes. Also fixed some mem leaks on error paths as I spotted them along the way. Reviewed-by: Tim Hudson --- apps/apps.c | 11 +++++++++++ apps/ca.c | 8 ++++++++ apps/cms.c | 4 ++++ apps/dgst.c | 5 +++++ apps/rsautl.c | 5 +++++ apps/s_cb.c | 5 +++++ apps/s_client.c | 5 +++++ apps/s_server.c | 20 +++++++++++++++++++- apps/speed.c | 12 ++++++++++-- apps/srp.c | 8 ++++++++ apps/x509.c | 5 +++++ crypto/asn1/bio_ndef.c | 6 ++++++ crypto/bio/b_print.c | 8 ++++++++ crypto/bio/bss_dgram.c | 15 ++++++++++++++- crypto/cms/cms_pwri.c | 2 ++ crypto/dh/dh_pmeth.c | 3 +++ crypto/dso/dso_vms.c | 3 ++- crypto/objects/o_names.c | 15 +++++++++------ crypto/rand/rand_os2.c | 3 +++ crypto/threads/th-lock.c | 23 ++++++++++++++++++++++- ssl/s3_pkt.c | 4 ++++ 21 files changed, 158 insertions(+), 12 deletions(-) diff --git a/apps/apps.c b/apps/apps.c index 8412e24687..233d382cd5 100644 --- a/apps/apps.c +++ b/apps/apps.c @@ -576,6 +576,11 @@ int password_callback(char *buf, int bufsiz, int verify, PW_CB_DATA *cb_tmp) char *prompt = NULL; prompt = UI_construct_prompt(ui, "pass phrase", prompt_info); + if(!prompt) { + BIO_printf(bio_err, "Out of memory\n"); + UI_free(ui); + return 0; + } ui_flags |= UI_INPUT_FLAG_DEFAULT_PWD; UI_ctrl(ui, UI_CTRL_PRINT_ERRORS, 1, 0, 0); @@ -585,6 +590,12 @@ int password_callback(char *buf, int bufsiz, int verify, PW_CB_DATA *cb_tmp) PW_MIN_LENGTH, bufsiz - 1); if (ok >= 0 && verify) { buff = (char *)OPENSSL_malloc(bufsiz); + if(!buff) { + BIO_printf(bio_err, "Out of memory\n"); + UI_free(ui); + OPENSSL_free(prompt); + return 0; + } ok = UI_add_verify_string(ui, prompt, ui_flags, buff, PW_MIN_LENGTH, bufsiz - 1, buf); } diff --git a/apps/ca.c b/apps/ca.c index bcb3f50d8a..814162d0b9 100644 --- a/apps/ca.c +++ b/apps/ca.c @@ -563,10 +563,18 @@ int MAIN(int argc, char **argv) #ifdef OPENSSL_SYS_VMS len = strlen(s) + sizeof(CONFIG_FILE); tofree = OPENSSL_malloc(len); + if(!tofree) { + BIO_printf(bio_err, "Out of memory\n"); + goto err; + } strcpy(tofree, s); #else len = strlen(s) + sizeof(CONFIG_FILE) + 1; tofree = OPENSSL_malloc(len); + if(!tofree) { + BIO_printf(bio_err, "Out of memory\n"); + goto err; + } BUF_strlcpy(tofree, s, len); BUF_strlcat(tofree, "/", len); #endif diff --git a/apps/cms.c b/apps/cms.c index 479d1dddf2..d983e28de9 100644 --- a/apps/cms.c +++ b/apps/cms.c @@ -465,6 +465,10 @@ int MAIN(int argc, char **argv) if (key_param == NULL || key_param->idx != keyidx) { cms_key_param *nparam; nparam = OPENSSL_malloc(sizeof(cms_key_param)); + if(!nparam) { + BIO_printf(bio_err, "Out of memory\n"); + goto argerr; + } nparam->idx = keyidx; nparam->param = sk_OPENSSL_STRING_new_null(); nparam->next = NULL; diff --git a/apps/dgst.c b/apps/dgst.c index adb7a060a2..47c2f69e1b 100644 --- a/apps/dgst.c +++ b/apps/dgst.c @@ -460,6 +460,11 @@ int MAIN(int argc, char **argv) ERR_print_errors(bio_err); goto end; } + if (!sigbuf) { + BIO_printf(bio_err, "Out of memory\n"); + ERR_print_errors(bio_err); + goto end; + } siglen = BIO_read(sigbio, sigbuf, siglen); BIO_free(sigbio); if (siglen <= 0) { diff --git a/apps/rsautl.c b/apps/rsautl.c index 0030aca126..d642f9ad97 100644 --- a/apps/rsautl.c +++ b/apps/rsautl.c @@ -268,6 +268,11 @@ int MAIN(int argc, char **argv) rsa_in = OPENSSL_malloc(keysize * 2); rsa_out = OPENSSL_malloc(keysize); + if (!rsa_in || !rsa_out) { + BIO_printf(bio_err, "Out of memory\n"); + ERR_print_errors(bio_err); + goto end; + } /* Read the input data */ rsa_inlen = BIO_read(in, rsa_in, keysize * 2); diff --git a/apps/s_cb.c b/apps/s_cb.c index eef86cb77b..12f7b8cb03 100644 --- a/apps/s_cb.c +++ b/apps/s_cb.c @@ -460,8 +460,13 @@ int ssl_print_curves(BIO *out, SSL *s, int noshared) if (ncurves <= 0) return 1; curves = OPENSSL_malloc(ncurves * sizeof(int)); + if(!curves) { + BIO_puts(out, "Malloc error getting supported curves\n"); + return 0; + } SSL_get1_curves(s, curves); + BIO_puts(out, "Supported Elliptic Curves: "); for (i = 0; i < ncurves; i++) { if (i) diff --git a/apps/s_client.c b/apps/s_client.c index bc82239f1b..3ec754f346 100644 --- a/apps/s_client.c +++ b/apps/s_client.c @@ -550,6 +550,11 @@ static char *ssl_give_srp_client_pwd_cb(SSL *s, void *arg) PW_CB_DATA cb_tmp; int l; + if(!pass) { + BIO_printf(bio_err, "Malloc failure\n"); + return NULL; + } + cb_tmp.password = (char *)srp_arg->srppassin; cb_tmp.prompt_info = "SRP user"; if ((l = password_callback(pass, PWD_STRLEN, 0, &cb_tmp)) < 0) { diff --git a/apps/s_server.c b/apps/s_server.c index 1792a3c1b1..cf5b50016f 100644 --- a/apps/s_server.c +++ b/apps/s_server.c @@ -649,6 +649,8 @@ static int ebcdic_new(BIO *bi) EBCDIC_OUTBUFF *wbuf; wbuf = (EBCDIC_OUTBUFF *) OPENSSL_malloc(sizeof(EBCDIC_OUTBUFF) + 1024); + if (!wbuf) + return 0; wbuf->alloced = 1024; wbuf->buff[0] = '\0'; @@ -703,9 +705,11 @@ static int ebcdic_write(BIO *b, const char *in, int inl) num = num + num; /* double the size */ if (num < inl) num = inl; - OPENSSL_free(wbuf); wbuf = (EBCDIC_OUTBUFF *) OPENSSL_malloc(sizeof(EBCDIC_OUTBUFF) + num); + if(!wbuf) + return 0; + OPENSSL_free(b->ptr); wbuf->alloced = num; wbuf->buff[0] = '\0'; @@ -3204,6 +3208,10 @@ static int add_session(SSL *ssl, SSL_SESSION *session) unsigned char *p; sess = OPENSSL_malloc(sizeof(simple_ssl_session)); + if(!sess) { + BIO_printf(bio_err, "Out of memory adding session to external cache\n"); + return 0; + } SSL_SESSION_get_id(session, &sess->idlen); sess->derlen = i2d_SSL_SESSION(session, NULL); @@ -3211,6 +3219,16 @@ static int add_session(SSL *ssl, SSL_SESSION *session) sess->id = BUF_memdup(SSL_SESSION_get_id(session, NULL), sess->idlen); sess->der = OPENSSL_malloc(sess->derlen); + if(!sess->id || !sess->der) { + BIO_printf(bio_err, "Out of memory adding session to external cache\n"); + + if(sess->id) + OPENSSL_free(sess->id); + if(sess->der) + OPENSSL_free(sess->der); + OPENSSL_free(sess); + return 0; + } p = sess->der; i2d_SSL_SESSION(session, &p); diff --git a/apps/speed.c b/apps/speed.c index ee9d2de706..57b53ce32e 100644 --- a/apps/speed.c +++ b/apps/speed.c @@ -2764,6 +2764,11 @@ static void multiblock_speed(const EVP_CIPHER *evp_cipher) inp = OPENSSL_malloc(mblengths[num - 1]); out = OPENSSL_malloc(mblengths[num - 1] + 1024); + if(!inp || !out) { + BIO_printf(bio_err,"Out of memory\n"); + goto end; + } + EVP_CIPHER_CTX_init(&ctx); EVP_EncryptInit_ex(&ctx, evp_cipher, NULL, no_key, no_iv); @@ -2848,6 +2853,9 @@ static void multiblock_speed(const EVP_CIPHER *evp_cipher) fprintf(stdout, "\n"); } - OPENSSL_free(inp); - OPENSSL_free(out); +end: + if(inp) + OPENSSL_free(inp); + if(out) + OPENSSL_free(out); } diff --git a/apps/srp.c b/apps/srp.c index e832535dd1..b9312f8dfc 100644 --- a/apps/srp.c +++ b/apps/srp.c @@ -437,10 +437,18 @@ int MAIN(int argc, char **argv) # ifdef OPENSSL_SYS_VMS len = strlen(s) + sizeof(CONFIG_FILE); tofree = OPENSSL_malloc(len); + if(!tofree) { + BIO_printf(bio_err, "Out of memory\n"); + goto err; + } strcpy(tofree, s); # else len = strlen(s) + sizeof(CONFIG_FILE) + 1; tofree = OPENSSL_malloc(len); + if(!tofree) { + BIO_printf(bio_err, "Out of memory\n"); + goto err; + } BUF_strlcpy(tofree, s, len); BUF_strlcat(tofree, "/", len); # endif diff --git a/apps/x509.c b/apps/x509.c index 4b08c181ee..380f0f0be7 100644 --- a/apps/x509.c +++ b/apps/x509.c @@ -819,6 +819,11 @@ int MAIN(int argc, char **argv) z = i2d_X509(x, NULL); m = OPENSSL_malloc(z); + if (!m) { + BIO_printf(bio_err, "Out of memory\n"); + ERR_print_errors(bio_err); + goto end; + } d = (unsigned char *)m; z = i2d_X509_NAME(X509_get_subject_name(x), &d); diff --git a/crypto/asn1/bio_ndef.c b/crypto/asn1/bio_ndef.c index 5817a2b8a7..4a73ca9eac 100644 --- a/crypto/asn1/bio_ndef.c +++ b/crypto/asn1/bio_ndef.c @@ -162,6 +162,9 @@ static int ndef_prefix(BIO *b, unsigned char **pbuf, int *plen, void *parg) derlen = ASN1_item_ndef_i2d(ndef_aux->val, NULL, ndef_aux->it); p = OPENSSL_malloc(derlen); + if(!p) + return 0; + ndef_aux->derbuf = p; *pbuf = p; derlen = ASN1_item_ndef_i2d(ndef_aux->val, &p, ndef_aux->it); @@ -229,6 +232,9 @@ static int ndef_suffix(BIO *b, unsigned char **pbuf, int *plen, void *parg) derlen = ASN1_item_ndef_i2d(ndef_aux->val, NULL, ndef_aux->it); p = OPENSSL_malloc(derlen); + if(!p) + return 0; + ndef_aux->derbuf = p; *pbuf = p; derlen = ASN1_item_ndef_i2d(ndef_aux->val, &p, ndef_aux->it); diff --git a/crypto/bio/b_print.c b/crypto/bio/b_print.c index 5dc7630009..f7940f28be 100644 --- a/crypto/bio/b_print.c +++ b/crypto/bio/b_print.c @@ -713,6 +713,10 @@ doapr_outch(char **sbuffer, if (*maxlen == 0) *maxlen = 1024; *buffer = OPENSSL_malloc(*maxlen); + if(!*buffer) { + /* Panic! Can't really do anything sensible. Just return */ + return; + } if (*currlen > 0) { assert(*sbuffer != NULL); memcpy(*buffer, *sbuffer, *currlen); @@ -721,6 +725,10 @@ doapr_outch(char **sbuffer, } else { *maxlen += 1024; *buffer = OPENSSL_realloc(*buffer, *maxlen); + if(!*buffer) { + /* Panic! Can't really do anything sensible. Just return */ + return; + } } } /* What to do if *buffer is NULL? */ diff --git a/crypto/bio/bss_dgram.c b/crypto/bio/bss_dgram.c index 0767809025..aef81499aa 100644 --- a/crypto/bio/bss_dgram.c +++ b/crypto/bio/bss_dgram.c @@ -998,6 +998,10 @@ BIO *BIO_new_dgram_sctp(int fd, int close_flag) */ sockopt_len = (socklen_t) (sizeof(sctp_assoc_t) + 256 * sizeof(uint8_t)); authchunks = OPENSSL_malloc(sockopt_len); + if(!authchunks) { + BIO_vfree(bio); + return (NULL); + } memset(authchunks, 0, sockopt_len); ret = getsockopt(fd, IPPROTO_SCTP, SCTP_LOCAL_AUTH_CHUNKS, authchunks, @@ -1333,6 +1337,10 @@ static int dgram_sctp_read(BIO *b, char *out, int outl) optlen = (socklen_t) (sizeof(sctp_assoc_t) + 256 * sizeof(uint8_t)); authchunks = OPENSSL_malloc(optlen); + if (!authchunks) { + BIOerr(BIO_F_DGRAM_SCTP_READ, ERR_R_MALLOC_ERROR); + return -1; + } memset(authchunks, 0, optlen); ii = getsockopt(b->num, IPPROTO_SCTP, SCTP_PEER_AUTH_CHUNKS, authchunks, &optlen); @@ -1399,10 +1407,15 @@ static int dgram_sctp_write(BIO *b, const char *in, int inl) * yet, we have to save it and send it as soon as the socket gets dry. */ if (data->save_shutdown && !BIO_dgram_sctp_wait_for_dry(b)) { + char *tmp; data->saved_message.bio = b; + if(!(tmp = OPENSSL_malloc(inl))) { + BIOerr(BIO_F_DGRAM_SCTP_WRITE, ERR_R_MALLOC_ERROR); + return -1; + } if (data->saved_message.data) OPENSSL_free(data->saved_message.data); - data->saved_message.data = OPENSSL_malloc(inl); + data->saved_message.data = tmp; memcpy(data->saved_message.data, in, inl); data->saved_message.length = inl; return inl; diff --git a/crypto/cms/cms_pwri.c b/crypto/cms/cms_pwri.c index 59691904e6..6729930920 100644 --- a/crypto/cms/cms_pwri.c +++ b/crypto/cms/cms_pwri.c @@ -231,6 +231,8 @@ static int kek_unwrap_key(unsigned char *out, size_t *outlen, return 0; } tmp = OPENSSL_malloc(inlen); + if(!tmp) + return 0; /* setup IV by decrypting last two blocks */ if (!EVP_DecryptUpdate(ctx, tmp + inlen - 2 * blocklen, &outl, in + inlen - 2 * blocklen, blocklen * 2) diff --git a/crypto/dh/dh_pmeth.c b/crypto/dh/dh_pmeth.c index c41de925c7..8975f4492a 100644 --- a/crypto/dh/dh_pmeth.c +++ b/crypto/dh/dh_pmeth.c @@ -468,6 +468,9 @@ static int pkey_dh_derive(EVP_PKEY_CTX *ctx, unsigned char *key, ret = 0; Zlen = DH_size(dh); Z = OPENSSL_malloc(Zlen); + if(!Z) { + goto err; + } if (DH_compute_key_padded(Z, dhpub, dh) <= 0) goto err; if (!DH_KDF_X9_42(key, *keylen, Z, Zlen, dctx->kdf_oid, diff --git a/crypto/dso/dso_vms.c b/crypto/dso/dso_vms.c index 6498184124..d3c4eab269 100644 --- a/crypto/dso/dso_vms.c +++ b/crypto/dso/dso_vms.c @@ -527,7 +527,8 @@ static char *vms_name_converter(DSO *dso, const char *filename) { int len = strlen(filename); char *not_translated = OPENSSL_malloc(len + 1); - strcpy(not_translated, filename); + if(not_translated) + strcpy(not_translated, filename); return (not_translated); } diff --git a/crypto/objects/o_names.c b/crypto/objects/o_names.c index d7fa0d853d..48ab1a7779 100644 --- a/crypto/objects/o_names.c +++ b/crypto/objects/o_names.c @@ -311,15 +311,18 @@ void OBJ_NAME_do_all_sorted(int type, d.type = type; d.names = OPENSSL_malloc(lh_OBJ_NAME_num_items(names_lh) * sizeof *d.names); - d.n = 0; - OBJ_NAME_do_all(type, do_all_sorted_fn, &d); + /* Really should return an error if !d.names...but its a void function! */ + if(d.names) { + d.n = 0; + OBJ_NAME_do_all(type, do_all_sorted_fn, &d); - qsort((void *)d.names, d.n, sizeof *d.names, do_all_sorted_cmp); + qsort((void *)d.names, d.n, sizeof *d.names, do_all_sorted_cmp); - for (n = 0; n < d.n; ++n) - fn(d.names[n], arg); + for (n = 0; n < d.n; ++n) + fn(d.names[n], arg); - OPENSSL_free((void *)d.names); + OPENSSL_free((void *)d.names); + } } static int free_type; diff --git a/crypto/rand/rand_os2.c b/crypto/rand/rand_os2.c index 9c4a137bb7..02148d5bf9 100644 --- a/crypto/rand/rand_os2.c +++ b/crypto/rand/rand_os2.c @@ -149,6 +149,9 @@ int RAND_poll(void) if (DosQuerySysState) { char *buffer = OPENSSL_malloc(256 * 1024); + if(!buffer) + return 0; + if (DosQuerySysState(0x1F, 0, 0, 0, buffer, 256 * 1024) == 0) { /* * First 4 bytes in buffer is a pointer to the thread count there diff --git a/crypto/threads/th-lock.c b/crypto/threads/th-lock.c index 9a8e90954e..7b303b281a 100644 --- a/crypto/threads/th-lock.c +++ b/crypto/threads/th-lock.c @@ -117,6 +117,10 @@ void CRYPTO_thread_setup(void) int i; lock_cs = OPENSSL_malloc(CRYPTO_num_locks() * sizeof(HANDLE)); + if(!lock_cs) { + /* Nothing we can do about this...void function! */ + return; + } for (i = 0; i < CRYPTO_num_locks(); i++) { lock_cs[i] = CreateMutex(NULL, FALSE, NULL); } @@ -168,6 +172,10 @@ void CRYPTO_thread_setup(void) # else lock_cs = OPENSSL_malloc(CRYPTO_num_locks() * sizeof(rwlock_t)); # endif + if(!lock_cs) { + /* Nothing we can do about this...void function! */ + return; + } lock_count = OPENSSL_malloc(CRYPTO_num_locks() * sizeof(long)); for (i = 0; i < CRYPTO_num_locks(); i++) { lock_count[i] = 0; @@ -239,6 +247,12 @@ void CRYPTO_thread_setup(void) int i; char filename[20]; + lock_cs = OPENSSL_malloc(CRYPTO_num_locks() * sizeof(usema_t *)); + if(!lock_cs) { + /* Nothing we can do about this...void function! */ + return; + } + strcpy(filename, "/tmp/mttest.XXXXXX"); mktemp(filename); @@ -249,7 +263,6 @@ void CRYPTO_thread_setup(void) arena = usinit(filename); unlink(filename); - lock_cs = OPENSSL_malloc(CRYPTO_num_locks() * sizeof(usema_t *)); for (i = 0; i < CRYPTO_num_locks(); i++) { lock_cs[i] = usnewsema(arena, 1); } @@ -303,6 +316,14 @@ void CRYPTO_thread_setup(void) lock_cs = OPENSSL_malloc(CRYPTO_num_locks() * sizeof(pthread_mutex_t)); lock_count = OPENSSL_malloc(CRYPTO_num_locks() * sizeof(long)); + if(!lock_cs || !lock_count) { + /* Nothing we can do about this...void function! */ + if(lock_cs) + OPENSSL_free(lock_cs); + if(lock_count) + OPENSSL_free(lock_count); + return; + } for (i = 0; i < CRYPTO_num_locks(); i++) { lock_count[i] = 0; pthread_mutex_init(&(lock_cs[i]), NULL); diff --git a/ssl/s3_pkt.c b/ssl/s3_pkt.c index 11b1c5532e..f54152e8ec 100644 --- a/ssl/s3_pkt.c +++ b/ssl/s3_pkt.c @@ -727,6 +727,10 @@ int ssl3_write_bytes(SSL *s, int type, const void *buf_, int len) packlen *= 4; wb->buf = OPENSSL_malloc(packlen); + if(!wb->buf) { + SSLerr(SSL_F_SSL3_WRITE_BYTES, ERR_R_MALLOC_FAILURE); + return -1; + } wb->len = packlen; } else if (tot == len) { /* done? */ OPENSSL_free(wb->buf); /* free jumbo buffer */ -- 2.34.1