From 385ece92d812d3ca154c9d86b0a58f2e9ff2181e Mon Sep 17 00:00:00 2001 From: Daniel Pouzzner Date: Fri, 11 Mar 2022 10:06:18 -0600 Subject: [PATCH] ECCSI and SAKKE: fix smallstackcache memory leaks in library, and blue-moon undefined behavior bugs in test.c eccsi_test(() and sakke_test(). --- wolfcrypt/src/eccsi.c | 13 +++++++ wolfcrypt/src/sakke.c | 18 +++++++++- wolfcrypt/test/test.c | 81 ++++++++++++++++++++++++++----------------- 3 files changed, 79 insertions(+), 33 deletions(-) diff --git a/wolfcrypt/src/eccsi.c b/wolfcrypt/src/eccsi.c index 57735dcc8..0f292f0fa 100644 --- a/wolfcrypt/src/eccsi.c +++ b/wolfcrypt/src/eccsi.c @@ -155,6 +155,7 @@ void wc_FreeEccsiKey(EccsiKey* key) wc_ecc_del_point_h(key->pvt, key->heap); wc_ecc_free(&key->pubkey); wc_ecc_free(&key->ecc); + XMEMSET(key, 0, sizeof(*key)); } } @@ -383,10 +384,12 @@ static int eccsi_compute_hs(EccsiKey* key, enum wc_HashType hashType, word32 dataSz = 0; int idx = wc_ecc_get_curve_idx(key->ecc.dp->id); ecc_point* kpak = &key->ecc.pubkey; + int hash_inited = 0; /* HS = hash( G | KPAK | ID | PVT ) */ err = wc_HashInit_ex(&key->hash, hashType, key->heap, INVALID_DEVID); if (err == 0) { + hash_inited = 1; /* Base Point - G */ dataSz = sizeof(key->data); err = eccsi_encode_base(key, key->data, &dataSz); @@ -426,6 +429,10 @@ static int eccsi_compute_hs(EccsiKey* key, enum wc_HashType hashType, *hashSz = (byte)wc_HashGetDigestSize(hashType); } + if (hash_inited) { + (void)wc_HashFree(&key->hash, hashType); + } + return err; } @@ -1774,10 +1781,12 @@ static int eccsi_compute_he(EccsiKey* key, enum wc_HashType hashType, { int err = 0; word32 dataSz = key->ecc.dp->size; + int hash_inited = 0; /* HE = hash( HS | r | M ) */ err = wc_HashInit_ex(&key->hash, hashType, key->heap, INVALID_DEVID); if (err == 0) { + hash_inited = 1; /* HS */ err = wc_HashUpdate(&key->hash, hashType, key->idHash, key->idHashSz); } @@ -1799,6 +1808,10 @@ static int eccsi_compute_he(EccsiKey* key, enum wc_HashType hashType, *heSz = wc_HashGetDigestSize(hashType); } + if (hash_inited) { + (void)wc_HashFree(&key->hash, hashType); + } + return err; } diff --git a/wolfcrypt/src/sakke.c b/wolfcrypt/src/sakke.c index 20dd033de..d09b9562a 100644 --- a/wolfcrypt/src/sakke.c +++ b/wolfcrypt/src/sakke.c @@ -6088,10 +6088,12 @@ static int sakke_calc_a(SakkeKey* key, enum wc_HashType hashType, const byte* data, word32 sz, const byte* extra, word32 extraSz, byte* a) { int err; + int hash_inited = 0; /* Step 1: A = hashfn( s ), where s = data | extra */ err = wc_HashInit_ex(&key->hash, hashType, key->heap, INVALID_DEVID); if (err == 0) { + hash_inited = 1; err = wc_HashUpdate(&key->hash, hashType, data, sz); } if ((err == 0) && (extra != NULL)) { @@ -6101,6 +6103,10 @@ static int sakke_calc_a(SakkeKey* key, enum wc_HashType hashType, err = wc_HashFinal(&key->hash, hashType, a); } + if (hash_inited) { + (void)wc_HashFree(&key->hash, hashType); + } + return err; } @@ -6127,13 +6133,19 @@ static int sakke_hash_to_range(SakkeKey* key, enum wc_HashType hashType, byte v[WC_MAX_DIGEST_SIZE]; word32 hashSz = 1; word32 i; + int hash_inited = 0; + + err = wc_HashInit_ex(&key->hash, hashType, key->heap, INVALID_DEVID); + if (err == 0) + hash_inited = 1; /* Step 1: A = hashfn( s ), where s = data | extra * See sakke_calc_a (need function parameters to be 7 or less) */ /* Step 2: h_0 = 00...00, a string of null bits of length hashlen bits */ - err = wc_HashGetDigestSize(hashType); + if (err == 0) + err = wc_HashGetDigestSize(hashType); if (err > 0) { hashSz = (word32)err; XMEMSET(h, 0, hashSz); @@ -6156,6 +6168,10 @@ static int sakke_hash_to_range(SakkeKey* key, enum wc_HashType hashType, } } + if (hash_inited) { + (void)wc_HashFree(&key->hash, hashType); + } + return err; } diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index 45c4b5325..e0e9c5505 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -28910,6 +28910,7 @@ int eccsi_test(void) { int ret = 0; WC_RNG rng; + int rng_inited = 0; EccsiKey* priv = NULL; EccsiKey* pub = NULL; mp_int* ssk = NULL; @@ -28917,24 +28918,27 @@ int eccsi_test(void) priv = (EccsiKey*)XMALLOC(sizeof(EccsiKey), HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - if (priv == NULL) { + if (priv == NULL) ret = -10205; - } + else + XMEMSET(priv, 0, sizeof(*priv)); if (ret == 0) { pub = (EccsiKey*)XMALLOC(sizeof(EccsiKey), HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - if (pub == NULL) { - ret = -10206; - } + if (pub == NULL) + ret = -10206; + else + XMEMSET(pub, 0, sizeof(*pub)); } if (ret == 0) { ssk = (mp_int*)XMALLOC(sizeof(mp_int), HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - if (ssk == NULL) { + if (ssk == NULL) ret = -10207; - } + else + XMEMSET(ssk, 0, sizeof(*ssk)); } if (ret == 0) { @@ -28945,6 +28949,8 @@ int eccsi_test(void) #endif if (ret != 0) ret = -10200; + else + rng_inited = 1; } if (ret == 0) { @@ -28987,19 +28993,22 @@ int eccsi_test(void) ret = eccsi_sign_verify_test(priv, pub, &rng, ssk, pvt); } - wc_FreeEccsiKey(priv); - wc_FreeEccsiKey(pub); - mp_free(ssk); - wc_ecc_del_point(pvt); - - if (ret != -10200) + if (pvt != NULL) + wc_ecc_del_point(pvt); + if (rng_inited) wc_FreeRng(&rng); - if (ssk != NULL) + if (ssk != NULL) { + mp_free(ssk); XFREE(ssk, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - if (pub != NULL) + } + if (pub != NULL) { + wc_FreeEccsiKey(pub); XFREE(pub, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - if (priv != NULL) + } + if (priv != NULL) { + wc_FreeEccsiKey(priv); XFREE(priv, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); + } return ret; } @@ -30056,6 +30065,7 @@ int sakke_test(void) { int ret = 0; WC_RNG rng; + int rng_inited = 0; SakkeKey* priv = NULL; SakkeKey* pub = NULL; SakkeKey* key = NULL; @@ -30063,24 +30073,27 @@ int sakke_test(void) priv = (SakkeKey*)XMALLOC(sizeof(SakkeKey), HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - if (priv == NULL) { + if (priv == NULL) ret = -10404; - } + else + XMEMSET(priv, 0, sizeof(*priv)); if (ret == 0) { pub = (SakkeKey*)XMALLOC(sizeof(SakkeKey), HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - if (pub == NULL) { + if (pub == NULL) ret = -10405; - } + else + XMEMSET(pub, 0, sizeof(*pub)); } if (ret == 0) { key = (SakkeKey*)XMALLOC(sizeof(SakkeKey), HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - if (key == NULL) { + if (key == NULL) ret = -10406; - } + else + XMEMSET(key, 0, sizeof(*key)); } if (ret == 0) { @@ -30089,7 +30102,9 @@ int sakke_test(void) #else ret = wc_InitRng(&rng); #endif - if (ret != 0) + if (ret == 0) + rng_inited = 1; + else ret = -10400; } @@ -30131,20 +30146,22 @@ int sakke_test(void) ret = sakke_op_test(priv, pub, &rng, rsk); } - wc_FreeSakkeKey(priv); - wc_FreeSakkeKey(pub); - wc_ecc_forcezero_point(rsk); - wc_ecc_del_point(rsk); - - if (ret != -10400) + if (rsk != NULL) { + wc_ecc_forcezero_point(rsk); + wc_ecc_del_point(rsk); + } + if (rng_inited) wc_FreeRng(&rng); - if (key != NULL) XFREE(key, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - if (pub != NULL) + if (pub != NULL) { + wc_FreeSakkeKey(pub); XFREE(pub, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - if (priv != NULL) + } + if (priv != NULL) { + wc_FreeSakkeKey(priv); XFREE(priv, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); + } return ret; }