From 67f74525e825e62056bc1fe034a67b143f292f47 Mon Sep 17 00:00:00 2001 From: David Garske Date: Fri, 23 Aug 2024 14:09:43 -0700 Subject: [PATCH] Fix possible leak in ED25519 because the `isAllocated` bit was being cleared before check. --- wolfcrypt/src/curve25519.c | 10 ++++++++-- wolfcrypt/src/ed25519.c | 9 +++++++-- wolfcrypt/src/rsa.c | 8 ++++++-- wolfssl/wolfcrypt/ed25519.h | 4 ++-- wolfssl/wolfcrypt/ed448.h | 2 +- 5 files changed, 24 insertions(+), 9 deletions(-) diff --git a/wolfcrypt/src/curve25519.c b/wolfcrypt/src/curve25519.c index 301ee73e9c..304fa3b95f 100644 --- a/wolfcrypt/src/curve25519.c +++ b/wolfcrypt/src/curve25519.c @@ -707,9 +707,15 @@ int wc_curve25519_init(curve25519_key* key) /* Clean the memory of a key */ void wc_curve25519_free(curve25519_key* key) { + int isAllocated = 0; + void* heap; + if (key == NULL) return; + isAllocated = key->isAllocated; + heap = key->heap; + #ifdef WOLFSSL_SE050 se050_curve25519_free_key(key); #endif @@ -719,12 +725,12 @@ void wc_curve25519_free(curve25519_key* key) XMEMSET(&key->p, 0, sizeof(key->p)); key->pubSet = 0; key->privSet = 0; + #ifdef WOLFSSL_CHECK_MEM_ZERO wc_MemZero_Check(key, sizeof(curve25519_key)); #endif - if (key->isAllocated) { - void* heap = key->heap; + if (isAllocated) { XFREE(key, heap, DYNAMIC_TYPE_CURVE25519); (void)heap; } diff --git a/wolfcrypt/src/ed25519.c b/wolfcrypt/src/ed25519.c index ca06a589a5..363ecc4aae 100644 --- a/wolfcrypt/src/ed25519.c +++ b/wolfcrypt/src/ed25519.c @@ -1023,9 +1023,15 @@ int wc_ed25519_init(ed25519_key* key) /* clear memory of key */ void wc_ed25519_free(ed25519_key* key) { + int isAllocated = 0; + void* heap; + if (key == NULL) return; + isAllocated = key->isAllocated; + heap = key->heap; + #ifdef WOLFSSL_ED25519_PERSISTENT_SHA ed25519_hash_free(key, &key->sha); #endif @@ -1039,8 +1045,7 @@ void wc_ed25519_free(ed25519_key* key) wc_MemZero_Check(key, sizeof(ed25519_key)); #endif - if (key->isAllocated) { - void* heap = key->heap; + if (isAllocated) { XFREE(key, heap, DYNAMIC_TYPE_ED25519); (void)heap; } diff --git a/wolfcrypt/src/rsa.c b/wolfcrypt/src/rsa.c index b44da81b96..b34cd67e39 100644 --- a/wolfcrypt/src/rsa.c +++ b/wolfcrypt/src/rsa.c @@ -542,11 +542,16 @@ int wc_RsaGetKeyId(RsaKey* key, word32* keyId) int wc_FreeRsaKey(RsaKey* key) { int ret = 0; + int isAllocated = 0; + void* heap; if (key == NULL) { return BAD_FUNC_ARG; } + isAllocated = key->isAllocated; + heap = key->heap; + wc_RsaCleanup(key); #if defined(WOLFSSL_ASYNC_CRYPT) && defined(WC_ASYNC_ENABLE_RSA) @@ -610,8 +615,7 @@ int wc_FreeRsaKey(RsaKey* key) wc_fspsm_RsaKeyFree(key); #endif - if (key->isAllocated) { - void* heap = key->heap; + if (isAllocated) { XFREE(key, heap, DYNAMIC_TYPE_RSA); (void)heap; } diff --git a/wolfssl/wolfcrypt/ed25519.h b/wolfssl/wolfcrypt/ed25519.h index 26746d5c5c..354ed0f8e7 100644 --- a/wolfssl/wolfcrypt/ed25519.h +++ b/wolfssl/wolfcrypt/ed25519.h @@ -106,9 +106,9 @@ struct ed25519_key { void *heap; #ifdef WOLFSSL_ED25519_PERSISTENT_SHA wc_Sha512 sha; - int sha_clean_flag; + unsigned int sha_clean_flag : 1; #endif - unsigned int isAllocated:1; /* flag indicates if structure was allocated */ + unsigned int isAllocated : 1; /* flag indicates if structure was allocated */ }; #ifndef WC_ED25519KEY_TYPE_DEFINED diff --git a/wolfssl/wolfcrypt/ed448.h b/wolfssl/wolfcrypt/ed448.h index 1d12da87ae..c8ede51fec 100644 --- a/wolfssl/wolfcrypt/ed448.h +++ b/wolfssl/wolfcrypt/ed448.h @@ -97,7 +97,7 @@ struct ed448_key { void *heap; #ifdef WOLFSSL_ED448_PERSISTENT_SHA wc_Shake sha; - int sha_clean_flag; + unsigned int sha_clean_flag : 1; #endif };