From 1a55309207cb2edd012062379c82da45ba6bbe9d Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Thu, 5 Jan 2017 10:00:17 -0700 Subject: [PATCH 1/3] fix possible memory leak on error case with ASN1 INTEGER to BN function --- src/ssl.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index ff297b4f7..4d12a13bd 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -16570,8 +16570,17 @@ WOLFSSL_BIGNUM *wolfSSL_ASN1_INTEGER_to_BN(const WOLFSSL_ASN1_INTEGER *ai, return NULL; } - if (SetIndividualExternal(&bn, &mpi) != SSL_SUCCESS) { - return NULL; + /* SetIndividualExternal mallocs bn in the case that bn is NULL */ + if (bn == NULL) { + if (SetIndividualExternal(&bn, &mpi) != SSL_SUCCESS) { + wolfSSL_BN_free(bn); + return NULL; + } + } + else { + if (SetIndividualExternal(&bn, &mpi) != SSL_SUCCESS) { + return NULL; + } } return bn; From 1afb7e20db8876b22b7c774d0e2e01f61f5d29a2 Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Thu, 5 Jan 2017 13:49:07 -0700 Subject: [PATCH 2/3] fix for freeing copy of mpi in the case of not using fastmath --- src/ssl.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/ssl.c b/src/ssl.c index 4d12a13bd..8dc27190d 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -16502,6 +16502,10 @@ void wolfSSL_RSA_free(WOLFSSL_RSA* rsa) * declared and then not used. */ #if !defined(NO_ASN) || !defined(NO_DSA) || defined(HAVE_ECC) || \ (!defined(NO_RSA) && !defined(HAVE_USER_RSA) && !defined(HAVE_FAST_RSA)) +/* when calling SetIndividualExternal, mpi should be cleared by caller if no + * longer used. ie mp_clear(mpi). This is to free data when fastmath is + * disabled since a copy of mpi is made by this function and placed into bn. + */ static int SetIndividualExternal(WOLFSSL_BIGNUM** bn, mp_int* mpi) { WOLFSSL_MSG("Entering SetIndividualExternal"); @@ -16570,18 +16574,23 @@ WOLFSSL_BIGNUM *wolfSSL_ASN1_INTEGER_to_BN(const WOLFSSL_ASN1_INTEGER *ai, return NULL; } - /* SetIndividualExternal mallocs bn in the case that bn is NULL */ + /* SetIndividualExternal mallocs bn in the case that bn is NULL + * mp_clear needs called because mpi is copied and causes memory leak with + * --disable-fastmath */ if (bn == NULL) { if (SetIndividualExternal(&bn, &mpi) != SSL_SUCCESS) { wolfSSL_BN_free(bn); + mp_clear(&mpi); return NULL; } } else { if (SetIndividualExternal(&bn, &mpi) != SSL_SUCCESS) { + mp_clear(&mpi); return NULL; } } + mp_clear(&mpi); return bn; } From 36d34ce0692da86375be28076132a076590ebf6a Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Wed, 11 Jan 2017 14:53:32 -0700 Subject: [PATCH 3/3] free WOLFSSL_BN in SetIndividualExternal error case and simplify mpi_clear call --- src/ssl.c | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index 8dc27190d..23fb7e4fb 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -16508,6 +16508,8 @@ void wolfSSL_RSA_free(WOLFSSL_RSA* rsa) */ static int SetIndividualExternal(WOLFSSL_BIGNUM** bn, mp_int* mpi) { + byte dynamic = 0; + WOLFSSL_MSG("Entering SetIndividualExternal"); if (mpi == NULL || bn == NULL) { @@ -16521,10 +16523,14 @@ static int SetIndividualExternal(WOLFSSL_BIGNUM** bn, mp_int* mpi) WOLFSSL_MSG("SetIndividualExternal alloc failed"); return SSL_FATAL_ERROR; } + dynamic = 1; } if (mp_copy(mpi, (mp_int*)((*bn)->internal)) != MP_OKAY) { WOLFSSL_MSG("mp_copy error"); + if (dynamic == 1) { + wolfSSL_BN_free(*bn); + } return SSL_FATAL_ERROR; } @@ -16574,24 +16580,14 @@ WOLFSSL_BIGNUM *wolfSSL_ASN1_INTEGER_to_BN(const WOLFSSL_ASN1_INTEGER *ai, return NULL; } - /* SetIndividualExternal mallocs bn in the case that bn is NULL - * mp_clear needs called because mpi is copied and causes memory leak with + /* mp_clear needs called because mpi is copied and causes memory leak with * --disable-fastmath */ - if (bn == NULL) { - if (SetIndividualExternal(&bn, &mpi) != SSL_SUCCESS) { - wolfSSL_BN_free(bn); - mp_clear(&mpi); - return NULL; - } - } - else { - if (SetIndividualExternal(&bn, &mpi) != SSL_SUCCESS) { - mp_clear(&mpi); - return NULL; - } - } + ret = SetIndividualExternal(&bn, &mpi); mp_clear(&mpi); + if (ret != SSL_SUCCESS) { + return NULL; + } return bn; } #endif /* !NO_ASN */