From 2014d39254cbdc6a6a475d5e2e7723e4f1e71f96 Mon Sep 17 00:00:00 2001 From: Daniel Pouzzner Date: Tue, 20 Jul 2021 18:26:05 -0500 Subject: [PATCH] fixes for valgrind-detected leaks and undefined data accesses: wolfSSL_{SHA*,MD5}_Final (OpenSSL compat wrappers): call wc_*Free() on sha state that otherwise leaks when _SMALL_STACK_CACHE; test_wc_curve25519_shared_secret_ex(): properly initialize public_key. --- src/ssl.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ tests/api.c | 19 +++++++++++++++++-- 2 files changed, 67 insertions(+), 2 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index 27317ff07..5a418244a 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -17732,6 +17732,11 @@ size_t wolfSSL_get_client_random(const WOLFSSL* ssl, unsigned char* out, WOLFSSL_ENTER("MD5_Final"); ret = wc_Md5Final((wc_Md5*)md5, input); + /* have to actually free the resources (if any) here, because the + * OpenSSL API doesn't include SHA*_Free(). + */ + wc_Md5Free((wc_Md5*)md5); + /* return 1 on success, 0 otherwise */ if (ret == 0) return 1; @@ -17808,6 +17813,11 @@ size_t wolfSSL_get_client_random(const WOLFSSL* ssl, unsigned char* out, WOLFSSL_ENTER("SHA_Final"); ret = wc_ShaFinal((wc_Sha*)sha, input); + /* have to actually free the resources (if any) here, because the + * OpenSSL API doesn't include SHA*_Free(). + */ + wc_ShaFree((wc_Sha*)sha); + /* return 1 on success, 0 otherwise */ if (ret == 0) return 1; @@ -17922,6 +17932,11 @@ size_t wolfSSL_get_client_random(const WOLFSSL* ssl, unsigned char* out, WOLFSSL_ENTER("SHA224_Final"); ret = wc_Sha224Final((wc_Sha224*)sha, input); + /* have to actually free the resources (if any) here, because the + * OpenSSL API doesn't include SHA*_Free(). + */ + wc_Sha224Free((wc_Sha224*)sha); + /* return 1 on success, 0 otherwise */ if (ret == 0) return 1; @@ -17973,6 +17988,11 @@ size_t wolfSSL_get_client_random(const WOLFSSL* ssl, unsigned char* out, WOLFSSL_ENTER("SHA256_Final"); ret = wc_Sha256Final((wc_Sha256*)sha, input); + /* have to actually free the resources (if any) here, because the + * OpenSSL API doesn't include SHA*_Free(). + */ + wc_Sha256Free((wc_Sha256*)sha); + /* return 1 on success, 0 otherwise */ if (ret == 0) return 1; @@ -18053,6 +18073,11 @@ size_t wolfSSL_get_client_random(const WOLFSSL* ssl, unsigned char* out, WOLFSSL_ENTER("SHA384_Final"); ret = wc_Sha384Final((wc_Sha384*)sha, input); + /* have to actually free the resources (if any) here, because the + * OpenSSL API doesn't include SHA*_Free(). + */ + wc_Sha384Free((wc_Sha384*)sha); + /* return 1 on success, 0 otherwise */ if (ret == 0) return 1; @@ -18106,6 +18131,11 @@ size_t wolfSSL_get_client_random(const WOLFSSL* ssl, unsigned char* out, WOLFSSL_ENTER("SHA512_Final"); ret = wc_Sha512Final((wc_Sha512*)sha, input); + /* have to actually free the resources (if any) here, because the + * OpenSSL API doesn't include SHA*_Free(). + */ + wc_Sha512Free((wc_Sha512*)sha); + /* return 1 on success, 0 otherwise */ if (ret == 0) return 1; @@ -18183,6 +18213,11 @@ size_t wolfSSL_get_client_random(const WOLFSSL* ssl, unsigned char* out, WOLFSSL_ENTER("SHA3_224_Final"); ret = wc_Sha3_224_Final((wc_Sha3*)sha, input); + /* have to actually free the resources (if any) here, because the + * OpenSSL API doesn't include SHA*_Free(). + */ + wc_Sha3_224_Free((wc_Sha3*)sha); + /* return 1 on success, 0 otherwise */ if (ret == 0) return 1; @@ -18235,6 +18270,11 @@ size_t wolfSSL_get_client_random(const WOLFSSL* ssl, unsigned char* out, WOLFSSL_ENTER("SHA3_256_Final"); ret = wc_Sha3_256_Final((wc_Sha3*)sha, input); + /* have to actually free the resources (if any) here, because the + * OpenSSL API doesn't include SHA*_Free(). + */ + wc_Sha3_256_Free((wc_Sha3*)sha); + /* return 1 on success, 0 otherwise */ if (ret == 0) return 1; @@ -18285,6 +18325,11 @@ size_t wolfSSL_get_client_random(const WOLFSSL* ssl, unsigned char* out, WOLFSSL_ENTER("SHA3_384_Final"); ret = wc_Sha3_384_Final((wc_Sha3*)sha, input); + /* have to actually free the resources (if any) here, because the + * OpenSSL API doesn't include SHA*_Free(). + */ + wc_Sha3_384_Free((wc_Sha3*)sha); + /* return 1 on success, 0 otherwise */ if (ret == 0) return 1; @@ -18337,6 +18382,11 @@ size_t wolfSSL_get_client_random(const WOLFSSL* ssl, unsigned char* out, WOLFSSL_ENTER("SHA3_512_Final"); ret = wc_Sha3_512_Final((wc_Sha3*)sha, input); + /* have to actually free the resources (if any) here, because the + * OpenSSL API doesn't include SHA*_Free(). + */ + wc_Sha3_512_Free((wc_Sha3*)sha); + /* return 1 on success, 0 otherwise */ if (ret == 0) return 1; diff --git a/tests/api.c b/tests/api.c index dd878071f..c404d42df 100644 --- a/tests/api.c +++ b/tests/api.c @@ -20023,6 +20023,9 @@ static int test_wc_curve25519_shared_secret_ex(void) printf(testingFmt, "wc_curve25519_shared_secret_ex()"); ret = wc_curve25519_init(&private_key); + if (ret == 0) { + ret = wc_curve25519_init(&public_key); + } if (ret == 0) { ret = wc_InitRng(&rng); } @@ -36564,6 +36567,8 @@ static void test_wolfSSL_SHA_Transform(void) AssertIntEQ(SHA_Transform(&sha, (const byte*)&local[0]), 1); AssertIntEQ(XMEMCMP(&((wc_Sha*)&sha)->digest[0], output1, WC_SHA_DIGEST_SIZE), 0); + AssertIntEQ(SHA_Final(local, &sha), 1); /* frees resources */ + /* Init SHA CTX */ AssertIntEQ(SHA_Init(&sha), 1); sLen = (word32)XSTRLEN((char*)input2); @@ -36572,6 +36577,8 @@ static void test_wolfSSL_SHA_Transform(void) AssertIntEQ(SHA_Transform(&sha, (const byte*)&local[0]), 1); AssertIntEQ(XMEMCMP(&((wc_Sha*)&sha)->digest[0], output2, WC_SHA_DIGEST_SIZE), 0); + AssertIntEQ(SHA_Final(local, &sha), 1); /* frees resources */ + /* SHA1 */ XMEMSET(local, 0, WC_SHA_BLOCK_SIZE); /* Init SHA CTX */ @@ -36582,6 +36589,8 @@ static void test_wolfSSL_SHA_Transform(void) AssertIntEQ(SHA1_Transform(&sha1, (const byte*)&local[0]), 1); AssertIntEQ(XMEMCMP(&((wc_Sha*)&sha1)->digest[0], output1, WC_SHA_DIGEST_SIZE), 0); + AssertIntEQ(SHA_Final(local, &sha), 1); /* frees resources */ + /* Init SHA CTX */ AssertIntEQ(SHA1_Init(&sha1), 1); sLen = (word32)XSTRLEN((char*)input2); @@ -36590,7 +36599,8 @@ static void test_wolfSSL_SHA_Transform(void) AssertIntEQ(SHA1_Transform(&sha1, (const byte*)&local[0]), 1); AssertIntEQ(XMEMCMP(&((wc_Sha*)&sha1)->digest[0], output2, WC_SHA_DIGEST_SIZE), 0); - + AssertIntEQ(SHA_Final(local, &sha), 1); /* frees resources */ + printf(resultFmt, passed); #endif #endif @@ -36643,6 +36653,7 @@ static void test_wolfSSL_SHA256_Transform(void) AssertIntEQ(SHA256_Transform(&sha256, (const byte*)&local[0]), 1); AssertIntEQ(XMEMCMP(&((wc_Sha256*)&sha256)->digest[0], output1, WC_SHA256_DIGEST_SIZE), 0); + AssertIntEQ(SHA256_Final(local, &sha256), 1); /* frees resources */ /* Init SHA256 CTX */ AssertIntEQ(SHA256_Init(&sha256), 1); @@ -36652,6 +36663,7 @@ static void test_wolfSSL_SHA256_Transform(void) AssertIntEQ(SHA256_Transform(&sha256, (const byte*)&local[0]), 1); AssertIntEQ(XMEMCMP(&((wc_Sha256*)&sha256)->digest[0], output2, WC_SHA256_DIGEST_SIZE), 0); + AssertIntEQ(SHA256_Final(local, &sha256), 1); /* frees resources */ printf(resultFmt, passed); #endif @@ -36736,8 +36748,9 @@ static void test_wolfSSL_SHA512_Transform(void) sLen = (word32)XSTRLEN((char*)input1); XMEMCPY(local, input1, sLen); AssertIntEQ(SHA512_Transform(&sha512, (const byte*)&local[0]), 1); - AssertIntEQ(XMEMCMP(&((wc_Sha512*)&sha512)->digest[0], output1, + AssertIntEQ(XMEMCMP(&((wc_Sha512*)&sha512)->digest[0], output1, WC_SHA512_DIGEST_SIZE), 0); + AssertIntEQ(SHA512_Final(local, &sha512), 1); /* frees resources */ /* Init SHA512 CTX */ AssertIntEQ(SHA512_Init(&sha512), 1); @@ -36747,6 +36760,8 @@ static void test_wolfSSL_SHA512_Transform(void) AssertIntEQ(SHA512_Transform(&sha512, (const byte*)&local[0]), 1); AssertIntEQ(XMEMCMP(&((wc_Sha512*)&sha512)->digest[0], output2, WC_SHA512_DIGEST_SIZE), 0); + AssertIntEQ(SHA512_Final(local, &sha512), 1); /* frees resources */ + (void)input1; printf(resultFmt, passed); #endif