<div dir="ltr"><div>Dear OpenSSL developers,</div><div><br></div><div>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</div><div>various cryptographic libraries and applications that use them.</div><div><br></div><div>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.</div><div><br></div><div>Please let us know how you intend to address these issues.</div><div><br></div><div>1:</div><div><a href="https://github.com/openssl/openssl/blob/master/apps/ts.c">https://github.com/openssl/openssl/blob/master/apps/ts.c</a></div><div>line 891, BIO_new_file(data, "rb") </div><div>bug info: memory leak on error path</div><div>patch:</div><div><br></div><div>--- a/apps/ts.c</div><div>+++ b/apps/ts.c</div><div>@@ -878,6 +878,7 @@ static TS_VERIFY_CTX *create_verify_ctx(const char *data, co</div><div> {</div><div>     TS_VERIFY_CTX *ctx = NULL;</div><div>     BIO *input = NULL;</div><div>+    BIO *out = NULL;</div><div>     TS_REQ *request = NULL;</div><div>     int ret = 0;</div><div>     int f = 0;</div><div>@@ -888,7 +889,8 @@ static TS_VERIFY_CTX *create_verify_ctx(const char *data, co</div><div>         f = TS_VFY_VERSION | TS_VFY_SIGNER;</div><div>         if (data != NULL) {</div><div>             f |= TS_VFY_DATA;</div><div>-            if (TS_VERIFY_CTX_set_data(ctx, BIO_new_file(data, "rb")) == NULL)</div><div>+            out = BIO_new_file(data, "rb")</div><div>+            if (TS_VERIFY_CTX_set_data(ctx, out) == NULL)</div><div>                 goto err;</div><div>         } else if (digest != NULL) {</div><div>             long imprint_len;</div><div>@@ -931,6 +933,7 @@ static TS_VERIFY_CTX *create_verify_ctx(const char *data, co</div><div>     }</div><div>     BIO_free_all(input);</div><div>     TS_REQ_free(request);</div><div>+    BIO_free_all(out)</div><div>     return ctx;</div><div> }</div><div><br></div><div><br></div><div><br></div><div>2:</div><div><a href="https://github.com/openssl/openssl/blob/master/crypto/dh/dh_gen.c">https://github.com/openssl/openssl/blob/master/crypto/dh/dh_gen.c</a></div><div>line 75,77,  ret->p = BN_new() </div><div>bug info: memory leak on error path</div><div>patch:</div><div>@@ -126,5 +126,7 @@ static int dh_builtin_genparams(DH *ret, int prime_len, int </div><div>         BN_CTX_end(ctx);</div><div>         BN_CTX_free(ctx);</div><div>     }</div><div>+    if(ret->p!=NULL)BN_free(ret->p);</div><div>+    if(ret->g!=NULL)BN_free(ret->g);</div><div>     return ok;</div><div> }</div><div><br></div><div><br></div><div>3:</div><div><a href="https://github.com/openssl/openssl/blob/master/crypto/ec/ec_key.c">https://github.com/openssl/openssl/blob/master/crypto/ec/ec_key.c</a></div><div>line 117, dest->priv_key = BN_new(); </div><div>bug info: memory leak on error path</div><div>patch:</div><div><br></div><div>@@ -119,9 +119,11 @@ EC_KEY *EC_KEY_copy(EC_KEY *dest, const EC_KEY *src)</div><div>                     return NULL;</div><div>             }</div><div>             if (!BN_copy(dest->priv_key, src->priv_key))</div><div>+                BN_free(dest->priv_key)</div><div>                 return NULL;</div><div>             if (src->group->meth->keycopy</div><div>                 && src->group->meth->keycopy(dest, src) == 0)</div><div>+                BN_free(dest->priv_key)</div><div>                 return NULL;</div><div>         }</div><div>     }</div><div>@@ -134,6 +136,7 @@ EC_KEY *EC_KEY_copy(EC_KEY *dest, const EC_KEY *src)</div><div>     dest->flags = src->flags;</div><div>     if (!CRYPTO_dup_ex_data(CRYPTO_EX_INDEX_EC_KEY,</div><div>                             &dest->ex_data, &src->ex_data))</div><div>+        BN_free(dest->priv_key)</div><div>         return NULL;</div><div> </div><div>     if (src->meth != dest->meth) {</div><div>@@ -146,6 +149,7 @@ EC_KEY *EC_KEY_copy(EC_KEY *dest, const EC_KEY *src)</div><div>     }</div><div> </div><div>     if (src->meth->copy != NULL && src->meth->copy(dest, src) == 0)</div><div>+        BN_free(dest->priv_key)</div><div>         return NULL;</div><div> </div><div>     return dest;</div><div><br></div><div><br></div><div>4:(solved in the recent commit)</div><div><a href="https://github.com/openssl/openssl/blob/master/crypto/asn1/a_digest.c">https://github.com/openssl/openssl/blob/master/crypto/asn1/a_digest.c</a></div><div>line 33, str = OPENSSL_malloc(i)); </div><div>bug info: memory leak on error path</div><div>patch: OPENSSL_free(str);</div><div>patch location: 41</div><div><br></div><div>5:</div><div><a href="https://github.com/openssl/openssl/blob/master/crypto/asn1/bio_ndef.c">https://github.com/openssl/openssl/blob/master/crypto/asn1/bio_ndef.c</a></div><div>line 116,185, p = OPENSSL_malloc(derlen);</div><div>bug info: memory leak on error path</div><div>patch:</div><div><br></div><div>@@ -122,6 +122,7 @@ static int ndef_prefix(BIO *b, unsigned char **pbuf, int *pl</div><div>     derlen = ASN1_item_ndef_i2d(ndef_aux->val, &p, ndef_aux->it);</div><div> </div><div>     if (!*ndef_aux->boundary)</div><div>+        OPENSSL_free(p);</div><div>         return 0;</div><div> </div><div>     *plen = *ndef_aux->boundary - *pbuf;</div><div>@@ -191,6 +192,7 @@ static int ndef_suffix(BIO *b, unsigned char **pbuf, int *pl</div><div>     derlen = ASN1_item_ndef_i2d(ndef_aux->val, &p, ndef_aux->it);</div><div> </div><div>     if (!*ndef_aux->boundary)</div><div>+        OPENSSL_free(p);</div><div>         return 0;</div><div>     *pbuf = *ndef_aux->boundary;</div><div>     *plen = derlen - (*ndef_aux->boundary - ndef_aux->derbuf);</div><div><br></div><div>6:</div><div><a href="https://github.com/openssl/openssl/blob/master/crypto/bio/bss_bio.c">https://github.com/openssl/openssl/blob/master/crypto/bio/bss_bio.c</a></div><div>line 625, b1->buf = OPENSSL_malloc(b1->size);</div><div>bug info: memory leak on error path</div><div>patch:</div><div><br></div><div>@@ -635,6 +635,7 @@ static int bio_make_pair(BIO *bio1, BIO *bio2)</div><div>         b2->buf = OPENSSL_malloc(b2->size);</div><div>         if (b2->buf == NULL) {</div><div>             BIOerr(BIO_F_BIO_MAKE_PAIR, ERR_R_MALLOC_FAILURE);</div><div>+            OPENSSL_free(b1->buf);</div><div>             return 0;</div><div>         }</div><div>         b2->len = 0;</div><div><br></div><div>7:</div><div><a href="https://github.com/openssl/openssl/blob/master/crypto/ec/ec_ameth.c">https://github.com/openssl/openssl/blob/master/crypto/ec/ec_ameth.c</a></div><div>line 244, ep = OPENSSL_malloc(eplen); </div><div>bug info: memory leak on error path</div><div>patch:</div><div>@@ -255,6 +255,7 @@ static int eckey_priv_encode(PKCS8_PRIV_KEY_INFO *p8, const </div><div> </div><div>     if (!PKCS8_pkey_set0(p8, OBJ_nid2obj(NID_X9_62_id_ecPublicKey), 0,</div><div>                          ptype, pval, ep, eplen))</div><div>+        OPENSSL_free(ep);</div><div>         return 0;</div><div> </div><div>     return 1;</div><div><br></div><div><br></div><div>8:</div><div><a href="https://github.com/openssl/openssl/blob/master/ssl/ssl_ciph.c">https://github.com/openssl/openssl/blob/master/ssl/ssl_ciph.c</a></div><div>line 1833, return 0;</div><div>bug info: Error propagation</div><div>patch:</div><div>@@ -1830,7 +1830,7 @@ int SSL_COMP_add_compression_method(int id, COMP_METHOD *c</div><div>     if (id < 193 || id > 255) {</div><div>         SSLerr(SSL_F_SSL_COMP_ADD_COMPRESSION_METHOD,</div><div>                SSL_R_COMPRESSION_ID_NOT_WITHIN_PRIVATE_RANGE);</div><div>-        return 0;</div><div>+        return 1;</div><div>     }</div><div> </div><div>     CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_DISABLE);</div><div><br></div><div><br></div><div>9:</div><div><a href="https://github.com/openssl/openssl/blob/master/ssl/t1_enc.c">https://github.com/openssl/openssl/blob/master/ssl/t1_enc.c</a></div><div>line 370, return (1);</div><div>bug info: Error propagation</div><div><br></div><div>line 388, p = OPENSSL_malloc(num)</div><div>bug info: memory leak on error path</div><div><br></div><div>patch:</div><div>@@ -367,7 +367,7 @@ int tls1_setup_key_block(SSL *s)</div><div>     int ret = 0;</div><div> </div><div>     if (s->s3->tmp.key_block_length != 0)</div><div>-        return (1);</div><div>+        return (0);</div><div> </div><div>     if (!ssl_cipher_get_evp</div><div>         (s->session, &c, &hash, &mac_type, &mac_secret_size, &comp,</div><div>@@ -448,6 +448,7 @@ int tls1_setup_key_block(SSL *s)</div><div> </div><div>     ret = 1;</div><div>  err:</div><div>+    OPENSSL_free(p);</div><div>     return (ret);</div><div> }</div><div><br></div><div><br></div><div><br></div><div>10:</div><div><a href="https://github.com/openssl/openssl/blob/master/ssl/s3_enc.c">https://github.com/openssl/openssl/blob/master/ssl/s3_enc.c</a></div><div>line 301, </div><div>bug info: missing check</div><div>line 270,</div><div>bug info: error propagation</div><div>line 295, </div><div>bug info: memory leak on error path</div><div>patch:</div><div><br></div><div>@@ -268,7 +268,7 @@ int ssl3_setup_key_block(SSL *s)</div><div>     SSL_COMP *comp;</div><div> </div><div>     if (s->s3->tmp.key_block_length != 0)</div><div>-        return (1);</div><div>+        return (0);</div><div> </div><div>     if (!ssl_cipher_get_evp(s->session, &c, &hash, NULL, NULL, &comp, 0)) {</div><div>         SSLerr(SSL_F_SSL3_SETUP_KEY_BLOCK, SSL_R_CIPHER_OR_HASH_UNAVAILABLE);</div><div>@@ -299,6 +299,7 @@ int ssl3_setup_key_block(SSL *s)</div><div>     s->s3->tmp.key_block = p;</div><div> </div><div>     ret = ssl3_generate_key_block(s, p, num);</div><div>+    if (ret==0) goto err;</div><div> </div><div>     if (!(s->options & SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS)) {</div><div>         /*</div><div>@@ -317,11 +318,12 @@ int ssl3_setup_key_block(SSL *s)</div><div> #endif</div><div>         }</div><div>     }</div><div>-</div><div>+    ret = 1</div><div>     return ret;</div><div> </div><div>  err:</div><div>     SSLerr(SSL_F_SSL3_SETUP_KEY_BLOCK, ERR_R_MALLOC_FAILURE);</div><div>+    OPENSSL_free(p);</div><div>     return (0);</div><div> }</div><div><br></div><div>Others: memory leak on error path</div><div>test/bntest.c</div><div>test/evp_test.c</div><div><br></div><div>Sincerely,</div><div>Baishakhi Ray, Yuchi Tian</div></div>