[openssl-dev] Bug reports and patches for OpenSSL
yuchi tian
yt8mn at virginia.edu
Sun Feb 5 21:07:58 UTC 2017
> 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.
Thank you for the information. I will submit the fixes as a github pull
request.
Sincerely,
Yuchi Tian
On Sun, Feb 5, 2017 at 8:49 AM, Matt Caswell <matt at openssl.org> wrote:
>
>
> 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
> >
> >
> --
> openssl-dev mailing list
> To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mta.openssl.org/pipermail/openssl-dev/attachments/20170205/934d7498/attachment-0001.html>
More information about the openssl-dev
mailing list