[openssl-dev] Bug reports and patches for OpenSSL
yuchi tian
yt8mn at virginia.edu
Sun Feb 5 06:54:06 UTC 2017
Dear OpenSSL developers,
We are software engineering researchers at University of Virginia. As part
of a research project, we have built a tool for automatically finding and
fixing error handling bugs and are testing it on
various cryptographic libraries and applications that use them.
In the most recent version of OpenSSL, we discovered various instances
where there may be memory leak on error path, potential error propagation
or missing check of function call. And we also give a patch for each
potential bug.
Please let us know how you intend to address these issues.
1:
https://github.com/openssl/openssl/blob/master/apps/ts.c
line 891, BIO_new_file(data, "rb")
bug info: memory leak on error path
patch:
--- a/apps/ts.c
+++ b/apps/ts.c
@@ -878,6 +878,7 @@ static TS_VERIFY_CTX *create_verify_ctx(const char
*data, co
{
TS_VERIFY_CTX *ctx = NULL;
BIO *input = NULL;
+ BIO *out = NULL;
TS_REQ *request = NULL;
int ret = 0;
int f = 0;
@@ -888,7 +889,8 @@ static TS_VERIFY_CTX *create_verify_ctx(const char
*data, co
f = TS_VFY_VERSION | TS_VFY_SIGNER;
if (data != NULL) {
f |= TS_VFY_DATA;
- if (TS_VERIFY_CTX_set_data(ctx, BIO_new_file(data, "rb")) ==
NULL)
+ out = BIO_new_file(data, "rb")
+ if (TS_VERIFY_CTX_set_data(ctx, out) == NULL)
goto err;
} else if (digest != NULL) {
long imprint_len;
@@ -931,6 +933,7 @@ static TS_VERIFY_CTX *create_verify_ctx(const char
*data, co
}
BIO_free_all(input);
TS_REQ_free(request);
+ BIO_free_all(out)
return ctx;
}
2:
https://github.com/openssl/openssl/blob/master/crypto/dh/dh_gen.c
line 75,77, ret->p = BN_new()
bug info: memory leak on error path
patch:
@@ -126,5 +126,7 @@ static int dh_builtin_genparams(DH *ret, int prime_len,
int
BN_CTX_end(ctx);
BN_CTX_free(ctx);
}
+ if(ret->p!=NULL)BN_free(ret->p);
+ if(ret->g!=NULL)BN_free(ret->g);
return ok;
}
3:
https://github.com/openssl/openssl/blob/master/crypto/ec/ec_key.c
line 117, dest->priv_key = BN_new();
bug info: memory leak on error path
patch:
@@ -119,9 +119,11 @@ EC_KEY *EC_KEY_copy(EC_KEY *dest, const EC_KEY *src)
return NULL;
}
if (!BN_copy(dest->priv_key, src->priv_key))
+ BN_free(dest->priv_key)
return NULL;
if (src->group->meth->keycopy
&& src->group->meth->keycopy(dest, src) == 0)
+ BN_free(dest->priv_key)
return NULL;
}
}
@@ -134,6 +136,7 @@ EC_KEY *EC_KEY_copy(EC_KEY *dest, const EC_KEY *src)
dest->flags = src->flags;
if (!CRYPTO_dup_ex_data(CRYPTO_EX_INDEX_EC_KEY,
&dest->ex_data, &src->ex_data))
+ BN_free(dest->priv_key)
return NULL;
if (src->meth != dest->meth) {
@@ -146,6 +149,7 @@ EC_KEY *EC_KEY_copy(EC_KEY *dest, const EC_KEY *src)
}
if (src->meth->copy != NULL && src->meth->copy(dest, src) == 0)
+ BN_free(dest->priv_key)
return NULL;
return dest;
4:(solved in the recent commit)
https://github.com/openssl/openssl/blob/master/crypto/asn1/a_digest.c
line 33, str = OPENSSL_malloc(i));
bug info: memory leak on error path
patch: OPENSSL_free(str);
patch location: 41
5:
https://github.com/openssl/openssl/blob/master/crypto/asn1/bio_ndef.c
line 116,185, p = OPENSSL_malloc(derlen);
bug info: memory leak on error path
patch:
@@ -122,6 +122,7 @@ static int ndef_prefix(BIO *b, unsigned char **pbuf,
int *pl
derlen = ASN1_item_ndef_i2d(ndef_aux->val, &p, ndef_aux->it);
if (!*ndef_aux->boundary)
+ OPENSSL_free(p);
return 0;
*plen = *ndef_aux->boundary - *pbuf;
@@ -191,6 +192,7 @@ static int ndef_suffix(BIO *b, unsigned char **pbuf,
int *pl
derlen = ASN1_item_ndef_i2d(ndef_aux->val, &p, ndef_aux->it);
if (!*ndef_aux->boundary)
+ OPENSSL_free(p);
return 0;
*pbuf = *ndef_aux->boundary;
*plen = derlen - (*ndef_aux->boundary - ndef_aux->derbuf);
6:
https://github.com/openssl/openssl/blob/master/crypto/bio/bss_bio.c
line 625, b1->buf = OPENSSL_malloc(b1->size);
bug info: memory leak on error path
patch:
@@ -635,6 +635,7 @@ static int bio_make_pair(BIO *bio1, BIO *bio2)
b2->buf = OPENSSL_malloc(b2->size);
if (b2->buf == NULL) {
BIOerr(BIO_F_BIO_MAKE_PAIR, ERR_R_MALLOC_FAILURE);
+ OPENSSL_free(b1->buf);
return 0;
}
b2->len = 0;
7:
https://github.com/openssl/openssl/blob/master/crypto/ec/ec_ameth.c
line 244, ep = OPENSSL_malloc(eplen);
bug info: memory leak on error path
patch:
@@ -255,6 +255,7 @@ static int eckey_priv_encode(PKCS8_PRIV_KEY_INFO *p8,
const
if (!PKCS8_pkey_set0(p8, OBJ_nid2obj(NID_X9_62_id_ecPublicKey), 0,
ptype, pval, ep, eplen))
+ OPENSSL_free(ep);
return 0;
return 1;
8:
https://github.com/openssl/openssl/blob/master/ssl/ssl_ciph.c
line 1833, return 0;
bug info: Error propagation
patch:
@@ -1830,7 +1830,7 @@ int SSL_COMP_add_compression_method(int id,
COMP_METHOD *c
if (id < 193 || id > 255) {
SSLerr(SSL_F_SSL_COMP_ADD_COMPRESSION_METHOD,
SSL_R_COMPRESSION_ID_NOT_WITHIN_PRIVATE_RANGE);
- return 0;
+ return 1;
}
CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_DISABLE);
9:
https://github.com/openssl/openssl/blob/master/ssl/t1_enc.c
line 370, return (1);
bug info: Error propagation
line 388, p = OPENSSL_malloc(num)
bug info: memory leak on error path
patch:
@@ -367,7 +367,7 @@ int tls1_setup_key_block(SSL *s)
int ret = 0;
if (s->s3->tmp.key_block_length != 0)
- return (1);
+ return (0);
if (!ssl_cipher_get_evp
(s->session, &c, &hash, &mac_type, &mac_secret_size, &comp,
@@ -448,6 +448,7 @@ int tls1_setup_key_block(SSL *s)
ret = 1;
err:
+ OPENSSL_free(p);
return (ret);
}
10:
https://github.com/openssl/openssl/blob/master/ssl/s3_enc.c
line 301,
bug info: missing check
line 270,
bug info: error propagation
line 295,
bug info: memory leak on error path
patch:
@@ -268,7 +268,7 @@ int ssl3_setup_key_block(SSL *s)
SSL_COMP *comp;
if (s->s3->tmp.key_block_length != 0)
- return (1);
+ return (0);
if (!ssl_cipher_get_evp(s->session, &c, &hash, NULL, NULL, &comp, 0)) {
SSLerr(SSL_F_SSL3_SETUP_KEY_BLOCK,
SSL_R_CIPHER_OR_HASH_UNAVAILABLE);
@@ -299,6 +299,7 @@ int ssl3_setup_key_block(SSL *s)
s->s3->tmp.key_block = p;
ret = ssl3_generate_key_block(s, p, num);
+ if (ret==0) goto err;
if (!(s->options & SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS)) {
/*
@@ -317,11 +318,12 @@ int ssl3_setup_key_block(SSL *s)
#endif
}
}
-
+ ret = 1
return ret;
err:
SSLerr(SSL_F_SSL3_SETUP_KEY_BLOCK, ERR_R_MALLOC_FAILURE);
+ OPENSSL_free(p);
return (0);
}
Others: memory leak on error path
test/bntest.c
test/evp_test.c
Sincerely,
Baishakhi Ray, Yuchi Tian
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mta.openssl.org/pipermail/openssl-dev/attachments/20170205/024df291/attachment.html>
More information about the openssl-dev
mailing list