<div dir="ltr"><div>> Guidance for how to correctly submit patches is given in the</div><div>> CONTRIBUTING file here:</div><div><br></div><div>> <a href="https://github.com/openssl/openssl/blob/master/CONTRIBUTING">https://github.com/openssl/openssl/blob/master/CONTRIBUTING</a></div><div><br></div><div>> Please could you submit your fixes as a github pull request? One pull</div><div>> request for all of these issues should be fine.</div><div><br></div><div>Thank you for the information. I will submit the fixes as a github pull request.</div><div><br></div><div>Sincerely,</div><div>Yuchi Tian</div><div><br></div><div class="gmail_extra"><br><div class="gmail_quote">On Sun, Feb 5, 2017 at 8:49 AM, Matt Caswell <span dir="ltr"><<a href="mailto:matt@openssl.org" target="_blank">matt@openssl.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
<br>
On 05/02/17 06:54, yuchi tian wrote:<br>
> Dear OpenSSL developers,<br>
><br>
> We are software engineering researchers at University of Virginia. As<br>
> part of a research project, we have built a tool for automatically<br>
> finding and fixing error handling bugs and are testing it on<br>
> various cryptographic libraries and applications that use them.<br>
><br>
> In the most recent version of OpenSSL, we discovered various instances<br>
> where there may be memory leak on error path, potential error<br>
> propagation or missing check of function call. And we also give a patch<br>
> for each potential bug.<br>
><br>
> Please let us know how you intend to address these issues.<br>
<br>
</span>Guidance for how to correctly submit patches is given in the<br>
CONTRIBUTING file here:<br>
<br>
<a href="https://github.com/openssl/openssl/blob/master/CONTRIBUTING" rel="noreferrer" target="_blank">https://github.com/openssl/<wbr>openssl/blob/master/<wbr>CONTRIBUTING</a><br>
<br>
Please could you submit your fixes as a github pull request? One pull<br>
request for all of these issues should be fine.<br>
<br>
We will also need a CLA from all authors:<br>
<a href="https://www.openssl.org/policies/cla.html" rel="noreferrer" target="_blank">https://www.openssl.org/<wbr>policies/cla.html</a><br>
<br>
Thanks!<br>
<span class="HOEnZb"><font color="#888888"><br>
Matt<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
<br>
<br>
><br>
> 1:<br>
> <a href="https://github.com/openssl/openssl/blob/master/apps/ts.c" rel="noreferrer" target="_blank">https://github.com/openssl/<wbr>openssl/blob/master/apps/ts.c</a><br>
> line 891, BIO_new_file(data, "rb")<br>
> bug info: memory leak on error path<br>
> patch:<br>
><br>
> --- a/apps/ts.c<br>
> +++ b/apps/ts.c<br>
> @@ -878,6 +878,7 @@ static TS_VERIFY_CTX *create_verify_ctx(const char<br>
> *data, co<br>
> {<br>
> TS_VERIFY_CTX *ctx = NULL;<br>
> BIO *input = NULL;<br>
> + BIO *out = NULL;<br>
> TS_REQ *request = NULL;<br>
> int ret = 0;<br>
> int f = 0;<br>
> @@ -888,7 +889,8 @@ static TS_VERIFY_CTX *create_verify_ctx(const char<br>
> *data, co<br>
> f = TS_VFY_VERSION | TS_VFY_SIGNER;<br>
> if (data != NULL) {<br>
> f |= TS_VFY_DATA;<br>
> - if (TS_VERIFY_CTX_set_data(ctx, BIO_new_file(data, "rb"))<br>
> == NULL)<br>
> + out = BIO_new_file(data, "rb")<br>
> + if (TS_VERIFY_CTX_set_data(ctx, out) == NULL)<br>
> goto err;<br>
> } else if (digest != NULL) {<br>
> long imprint_len;<br>
> @@ -931,6 +933,7 @@ static TS_VERIFY_CTX *create_verify_ctx(const char<br>
> *data, co<br>
> }<br>
> BIO_free_all(input);<br>
> TS_REQ_free(request);<br>
> + BIO_free_all(out)<br>
> return ctx;<br>
> }<br>
><br>
><br>
><br>
> 2:<br>
> <a href="https://github.com/openssl/openssl/blob/master/crypto/dh/dh_gen.c" rel="noreferrer" target="_blank">https://github.com/openssl/<wbr>openssl/blob/master/crypto/dh/<wbr>dh_gen.c</a><br>
> line 75,77, ret->p = BN_new()<br>
> bug info: memory leak on error path<br>
> patch:<br>
> @@ -126,5 +126,7 @@ static int dh_builtin_genparams(DH *ret, int<br>
> prime_len, int<br>
> BN_CTX_end(ctx);<br>
> BN_CTX_free(ctx);<br>
> }<br>
> + if(ret->p!=NULL)BN_free(ret-><wbr>p);<br>
> + if(ret->g!=NULL)BN_free(ret-><wbr>g);<br>
> return ok;<br>
> }<br>
><br>
><br>
> 3:<br>
> <a href="https://github.com/openssl/openssl/blob/master/crypto/ec/ec_key.c" rel="noreferrer" target="_blank">https://github.com/openssl/<wbr>openssl/blob/master/crypto/ec/<wbr>ec_key.c</a><br>
> line 117, dest->priv_key = BN_new();<br>
> bug info: memory leak on error path<br>
> patch:<br>
><br>
> @@ -119,9 +119,11 @@ EC_KEY *EC_KEY_copy(EC_KEY *dest, const EC_KEY *src)<br>
> return NULL;<br>
> }<br>
> if (!BN_copy(dest->priv_key, src->priv_key))<br>
> + BN_free(dest->priv_key)<br>
> return NULL;<br>
> if (src->group->meth->keycopy<br>
> && src->group->meth->keycopy(<wbr>dest, src) == 0)<br>
> + BN_free(dest->priv_key)<br>
> return NULL;<br>
> }<br>
> }<br>
> @@ -134,6 +136,7 @@ EC_KEY *EC_KEY_copy(EC_KEY *dest, const EC_KEY *src)<br>
> dest->flags = src->flags;<br>
> if (!CRYPTO_dup_ex_data(CRYPTO_<wbr>EX_INDEX_EC_KEY,<br>
> &dest->ex_data, &src->ex_data))<br>
> + BN_free(dest->priv_key)<br>
> return NULL;<br>
><br>
> if (src->meth != dest->meth) {<br>
> @@ -146,6 +149,7 @@ EC_KEY *EC_KEY_copy(EC_KEY *dest, const EC_KEY *src)<br>
> }<br>
><br>
> if (src->meth->copy != NULL && src->meth->copy(dest, src) == 0)<br>
> + BN_free(dest->priv_key)<br>
> return NULL;<br>
><br>
> return dest;<br>
><br>
><br>
> 4:(solved in the recent commit)<br>
> <a href="https://github.com/openssl/openssl/blob/master/crypto/asn1/a_digest.c" rel="noreferrer" target="_blank">https://github.com/openssl/<wbr>openssl/blob/master/crypto/<wbr>asn1/a_digest.c</a><br>
> line 33, str = OPENSSL_malloc(i));<br>
> bug info: memory leak on error path<br>
> patch: OPENSSL_free(str);<br>
> patch location: 41<br>
><br>
> 5:<br>
> <a href="https://github.com/openssl/openssl/blob/master/crypto/asn1/bio_ndef.c" rel="noreferrer" target="_blank">https://github.com/openssl/<wbr>openssl/blob/master/crypto/<wbr>asn1/bio_ndef.c</a><br>
> line 116,185, p = OPENSSL_malloc(derlen);<br>
> bug info: memory leak on error path<br>
> patch:<br>
><br>
> @@ -122,6 +122,7 @@ static int ndef_prefix(BIO *b, unsigned char **pbuf,<br>
> int *pl<br>
> derlen = ASN1_item_ndef_i2d(ndef_aux-><wbr>val, &p, ndef_aux->it);<br>
><br>
> if (!*ndef_aux->boundary)<br>
> + OPENSSL_free(p);<br>
> return 0;<br>
><br>
> *plen = *ndef_aux->boundary - *pbuf;<br>
> @@ -191,6 +192,7 @@ static int ndef_suffix(BIO *b, unsigned char **pbuf,<br>
> int *pl<br>
> derlen = ASN1_item_ndef_i2d(ndef_aux-><wbr>val, &p, ndef_aux->it);<br>
><br>
> if (!*ndef_aux->boundary)<br>
> + OPENSSL_free(p);<br>
> return 0;<br>
> *pbuf = *ndef_aux->boundary;<br>
> *plen = derlen - (*ndef_aux->boundary - ndef_aux->derbuf);<br>
><br>
> 6:<br>
> <a href="https://github.com/openssl/openssl/blob/master/crypto/bio/bss_bio.c" rel="noreferrer" target="_blank">https://github.com/openssl/<wbr>openssl/blob/master/crypto/<wbr>bio/bss_bio.c</a><br>
> line 625, b1->buf = OPENSSL_malloc(b1->size);<br>
> bug info: memory leak on error path<br>
> patch:<br>
><br>
> @@ -635,6 +635,7 @@ static int bio_make_pair(BIO *bio1, BIO *bio2)<br>
> b2->buf = OPENSSL_malloc(b2->size);<br>
> if (b2->buf == NULL) {<br>
> BIOerr(BIO_F_BIO_MAKE_PAIR, ERR_R_MALLOC_FAILURE);<br>
> + OPENSSL_free(b1->buf);<br>
> return 0;<br>
> }<br>
> b2->len = 0;<br>
><br>
> 7:<br>
> <a href="https://github.com/openssl/openssl/blob/master/crypto/ec/ec_ameth.c" rel="noreferrer" target="_blank">https://github.com/openssl/<wbr>openssl/blob/master/crypto/ec/<wbr>ec_ameth.c</a><br>
> line 244, ep = OPENSSL_malloc(eplen);<br>
> bug info: memory leak on error path<br>
> patch:<br>
> @@ -255,6 +255,7 @@ static int eckey_priv_encode(PKCS8_PRIV_<wbr>KEY_INFO<br>
> *p8, const<br>
><br>
> if (!PKCS8_pkey_set0(p8, OBJ_nid2obj(NID_X9_62_id_<wbr>ecPublicKey), 0,<br>
> ptype, pval, ep, eplen))<br>
> + OPENSSL_free(ep);<br>
> return 0;<br>
><br>
> return 1;<br>
><br>
><br>
> 8:<br>
> <a href="https://github.com/openssl/openssl/blob/master/ssl/ssl_ciph.c" rel="noreferrer" target="_blank">https://github.com/openssl/<wbr>openssl/blob/master/ssl/ssl_<wbr>ciph.c</a><br>
> line 1833, return 0;<br>
> bug info: Error propagation<br>
> patch:<br>
> @@ -1830,7 +1830,7 @@ int SSL_COMP_add_compression_<wbr>method(int id,<br>
> COMP_METHOD *c<br>
> if (id < 193 || id > 255) {<br>
> SSLerr(SSL_F_SSL_COMP_ADD_<wbr>COMPRESSION_METHOD,<br>
> SSL_R_COMPRESSION_ID_NOT_<wbr>WITHIN_PRIVATE_RANGE);<br>
> - return 0;<br>
> + return 1;<br>
> }<br>
><br>
> CRYPTO_mem_ctrl(CRYPTO_MEM_<wbr>CHECK_DISABLE);<br>
><br>
><br>
> 9:<br>
> <a href="https://github.com/openssl/openssl/blob/master/ssl/t1_enc.c" rel="noreferrer" target="_blank">https://github.com/openssl/<wbr>openssl/blob/master/ssl/t1_<wbr>enc.c</a><br>
> line 370, return (1);<br>
> bug info: Error propagation<br>
><br>
> line 388, p = OPENSSL_malloc(num)<br>
> bug info: memory leak on error path<br>
><br>
> patch:<br>
> @@ -367,7 +367,7 @@ int tls1_setup_key_block(SSL *s)<br>
> int ret = 0;<br>
><br>
> if (s->s3->tmp.key_block_length != 0)<br>
> - return (1);<br>
> + return (0);<br>
><br>
> if (!ssl_cipher_get_evp<br>
> (s->session, &c, &hash, &mac_type, &mac_secret_size, &comp,<br>
> @@ -448,6 +448,7 @@ int tls1_setup_key_block(SSL *s)<br>
><br>
> ret = 1;<br>
> err:<br>
> + OPENSSL_free(p);<br>
> return (ret);<br>
> }<br>
><br>
><br>
><br>
> 10:<br>
> <a href="https://github.com/openssl/openssl/blob/master/ssl/s3_enc.c" rel="noreferrer" target="_blank">https://github.com/openssl/<wbr>openssl/blob/master/ssl/s3_<wbr>enc.c</a><br>
> line 301,<br>
> bug info: missing check<br>
> line 270,<br>
> bug info: error propagation<br>
> line 295,<br>
> bug info: memory leak on error path<br>
> patch:<br>
><br>
> @@ -268,7 +268,7 @@ int ssl3_setup_key_block(SSL *s)<br>
> SSL_COMP *comp;<br>
><br>
> if (s->s3->tmp.key_block_length != 0)<br>
> - return (1);<br>
> + return (0);<br>
><br>
> if (!ssl_cipher_get_evp(s-><wbr>session, &c, &hash, NULL, NULL, &comp, 0)) {<br>
> SSLerr(SSL_F_SSL3_SETUP_KEY_<wbr>BLOCK,<br>
> SSL_R_CIPHER_OR_HASH_<wbr>UNAVAILABLE);<br>
> @@ -299,6 +299,7 @@ int ssl3_setup_key_block(SSL *s)<br>
> s->s3->tmp.key_block = p;<br>
><br>
> ret = ssl3_generate_key_block(s, p, num);<br>
> + if (ret==0) goto err;<br>
><br>
> if (!(s->options & SSL_OP_DONT_INSERT_EMPTY_<wbr>FRAGMENTS)) {<br>
> /*<br>
> @@ -317,11 +318,12 @@ int ssl3_setup_key_block(SSL *s)<br>
> #endif<br>
> }<br>
> }<br>
> -<br>
> + ret = 1<br>
> return ret;<br>
><br>
> err:<br>
> SSLerr(SSL_F_SSL3_SETUP_KEY_<wbr>BLOCK, ERR_R_MALLOC_FAILURE);<br>
> + OPENSSL_free(p);<br>
> return (0);<br>
> }<br>
><br>
> Others: memory leak on error path<br>
> test/bntest.c<br>
> test/evp_test.c<br>
><br>
> Sincerely,<br>
> Baishakhi Ray, Yuchi Tian<br>
><br>
><br>
</div></div><div class="HOEnZb"><div class="h5">--<br>
openssl-dev mailing list<br>
To unsubscribe: <a href="https://mta.openssl.org/mailman/listinfo/openssl-dev" rel="noreferrer" target="_blank">https://mta.openssl.org/<wbr>mailman/listinfo/openssl-dev</a><br>
</div></div></blockquote></div><br></div></div>