[openssl-dev] Bug reports and patches for OpenSSL
Matt Caswell
matt at openssl.org
Sun Feb 5 13:49:21 UTC 2017
On 05/02/17 06:54, yuchi tian wrote:
> 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.
Guidance for how to correctly submit patches is given in the
CONTRIBUTING file here:
https://github.com/openssl/openssl/blob/master/CONTRIBUTING
Please could you submit your fixes as a github pull request? One pull
request for all of these issues should be fine.
We will also need a CLA from all authors:
https://www.openssl.org/policies/cla.html
Thanks!
Matt
>
> 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
>
>
More information about the openssl-dev
mailing list