Skip to content

Commit 921ecf0

Browse files
JacobBarthelmehJacobBarthelmeh
authored
Merge pull request #9830 from aidangarske/fix-wolfcrypt-fenrir
Fix fenrir security findings for wolfcrypt
2 parents 85228f0 + 3295a65 commit 921ecf0

11 files changed

Lines changed: 55 additions & 16 deletions

File tree

‎wolfcrypt/src/aes.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14808,7 +14808,7 @@ int wc_AesKeyUnWrap_ex(Aes *aes, const byte* in, word32 inSz, byte* out,
1480814808
return ret;
1480914809

1481014810
/* verify IV */
14811-
if (XMEMCMP(tmp, expIv, KEYWRAP_BLOCK_SIZE) != 0)
14811+
if (ConstantCompare(tmp, expIv, KEYWRAP_BLOCK_SIZE) != 0)
1481214812
return BAD_KEYWRAP_IV_E;
1481314813

1481414814
return (int)(inSz - KEYWRAP_BLOCK_SIZE);
@@ -16507,7 +16507,7 @@ static WARN_UNUSED_RESULT int AesSivCipher(
1650716507
WOLFSSL_MSG("S2V failed.");
1650816508
}
1650916509

16510-
if (XMEMCMP(siv, sivTmp, WC_AES_BLOCK_SIZE) != 0) {
16510+
if (ConstantCompare(siv, sivTmp, WC_AES_BLOCK_SIZE) != 0) {
1651116511
WOLFSSL_MSG("Computed SIV doesn't match received SIV.");
1651216512
ret = AES_SIV_AUTH_E;
1651316513
}

‎wolfcrypt/src/asn.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -487,7 +487,7 @@ static word32 SizeASNLength(word32 length)
487487
#define ALLOC_ASNSETDATA(name, cnt, err, heap) \
488488
do { \
489489
if ((err) == 0) { \
490-
(name) = (ASNSetData*)XMALLOC(sizeof(ASNGetData) * (cnt), (heap), \
490+
(name) = (ASNSetData*)XMALLOC(sizeof(ASNSetData) * (cnt), (heap), \
491491
DYNAMIC_TYPE_TMP_BUFFER); \
492492
if ((name) == NULL) { \
493493
(err) = MEMORY_E; \

‎wolfcrypt/src/chacha20_poly1305.c‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,8 @@ int wc_ChaCha20Poly1305_Init(ChaChaPoly_Aead* aead,
187187
aead->state = CHACHA20_POLY1305_STATE_READY;
188188
}
189189

190+
ForceZero(authKey, sizeof(authKey));
191+
190192
return ret;
191193
}
192194

@@ -332,25 +334,30 @@ int wc_XChaCha20Poly1305_Init(
332334
/* Create the Poly1305 key */
333335
if ((ret = wc_Chacha_Process(&aead->chacha, authKey, authKey,
334336
(word32)sizeof authKey)) < 0)
335-
return ret;
337+
goto out;
336338
/* advance to start of the next ChaCha block. */
337339
wc_Chacha_purge_current_block(&aead->chacha);
338340

339341
/* Initialize Poly1305 context */
340342
if ((ret = wc_Poly1305SetKey(&aead->poly, authKey,
341343
(word32)sizeof authKey)) < 0)
342-
return ret;
344+
goto out;
343345

344346
if ((ret = wc_Poly1305Update(&aead->poly, ad, (word32)ad_len)) < 0)
345-
return ret;
347+
goto out;
346348

347349
if ((ret = wc_Poly1305_Pad(&aead->poly, (word32)ad_len)) < 0)
348-
return ret;
350+
goto out;
349351

350352
aead->isEncrypt = isEncrypt ? 1 : 0;
351353
aead->state = CHACHA20_POLY1305_STATE_AAD;
352354

353-
return 0;
355+
ret = 0;
356+
357+
out:
358+
ForceZero(authKey, sizeof(authKey));
359+
360+
return ret;
354361
}
355362

356363
static WC_INLINE int wc_XChaCha20Poly1305_crypt_oneshot(

‎wolfcrypt/src/ecc.c‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14484,6 +14484,8 @@ int wc_ecc_encrypt_ex(ecc_key* privKey, ecc_key* pubKey, const byte* msg,
1448414484

1448514485
RESTORE_VECTOR_REGISTERS();
1448614486

14487+
ForceZero(sharedSecret, sharedSz);
14488+
ForceZero(keys, (word32)keysLen);
1448714489
WC_FREE_VAR_EX(sharedSecret, ctx->heap, DYNAMIC_TYPE_ECC_BUFFER);
1448814490
WC_FREE_VAR_EX(keys, ctx->heap, DYNAMIC_TYPE_ECC_BUFFER);
1448914491

@@ -14778,8 +14780,8 @@ int wc_ecc_decrypt(ecc_key* privKey, ecc_key* pubKey, const byte* msg,
1477814780

1477914781
if (ret == 0)
1478014782
ret = wc_HmacFinal(hmac, verify);
14781-
if ((ret == 0) && (XMEMCMP(verify, msg + msgSz - digestSz,
14782-
digestSz) != 0)) {
14783+
if ((ret == 0) && (ConstantCompare(verify, msg + msgSz - digestSz,
14784+
(int)digestSz) != 0)) {
1478314785
ret = HASH_TYPE_E;
1478414786
WOLFSSL_MSG("ECC Decrypt HMAC Check failed!");
1478514787
}
@@ -14882,6 +14884,8 @@ int wc_ecc_decrypt(ecc_key* privKey, ecc_key* pubKey, const byte* msg,
1488214884
if (pubKey == peerKey)
1488314885
wc_ecc_free(peerKey);
1488414886
#endif
14887+
ForceZero(sharedSecret, sharedSz);
14888+
ForceZero(keys, (word32)keysLen);
1488514889
#ifdef WOLFSSL_SMALL_STACK
1488614890
#ifndef WOLFSSL_ECIES_OLD
1488714891
XFREE(peerKey, ctx->heap, DYNAMIC_TYPE_ECC_BUFFER);

‎wolfcrypt/src/evp.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4952,7 +4952,7 @@ int wolfSSL_EVP_DigestVerifyFinal(WOLFSSL_EVP_MD_CTX *ctx,
49524952

49534953
hashLen = wolfssl_mac_len(ctx->hash.hmac.macType);
49544954

4955-
if (siglen > hashLen)
4955+
if (siglen > hashLen || siglen > INT_MAX)
49564956
return WOLFSSL_FAILURE;
49574957
/* May be a truncated signature. */
49584958
}
@@ -4962,7 +4962,7 @@ int wolfSSL_EVP_DigestVerifyFinal(WOLFSSL_EVP_MD_CTX *ctx,
49624962

49634963
if (ctx->isHMAC) {
49644964
/* Check HMAC result matches the signature. */
4965-
if (XMEMCMP(sig, digest, (size_t)siglen) == 0)
4965+
if (ConstantCompare(sig, digest, (int)siglen) == 0)
49664966
return WOLFSSL_SUCCESS;
49674967
return WOLFSSL_FAILURE;
49684968
}

‎wolfcrypt/src/hpke.c‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -796,6 +796,8 @@ static int wc_HpkeEncap(Hpke* hpke, void* ephemeralKey, void* receiverKey,
796796
hpke->Npk * 2, sharedSecret);
797797
}
798798

799+
ForceZero(dh, hpke->Ndh);
800+
ForceZero(kemContext, hpke->Npk * 2);
799801
WC_FREE_VAR_EX(dh, hpke->heap, DYNAMIC_TYPE_TMP_BUFFER);
800802
WC_FREE_VAR_EX(kemContext, hpke->heap, DYNAMIC_TYPE_TMP_BUFFER);
801803

@@ -816,6 +818,9 @@ static int wc_HpkeSetupBaseSender(Hpke* hpke, HpkeBaseContext* context,
816818
#ifdef WOLFSSL_SMALL_STACK
817819
sharedSecret = (byte*)XMALLOC(hpke->Nsecret, hpke->heap,
818820
DYNAMIC_TYPE_TMP_BUFFER);
821+
if (sharedSecret == NULL) {
822+
return MEMORY_E;
823+
}
819824
#endif
820825

821826
/* encap */
@@ -827,6 +832,7 @@ static int wc_HpkeSetupBaseSender(Hpke* hpke, HpkeBaseContext* context,
827832
infoSz);
828833
}
829834

835+
ForceZero(sharedSecret, hpke->Nsecret);
830836
WC_FREE_VAR_EX(sharedSecret, hpke->heap, DYNAMIC_TYPE_TMP_BUFFER);
831837

832838
return ret;
@@ -914,6 +920,7 @@ int wc_HpkeSealBase(Hpke* hpke, void* ephemeralKey, void* receiverKey,
914920

915921
PRIVATE_KEY_LOCK();
916922

923+
ForceZero(context, sizeof(HpkeBaseContext));
917924
WC_FREE_VAR_EX(context, hpke->heap, DYNAMIC_TYPE_TMP_BUFFER);
918925

919926
return ret;
@@ -1032,6 +1039,8 @@ static int wc_HpkeDecap(Hpke* hpke, void* receiverKey, const byte* pubKey,
10321039
hpke->Npk * 2, sharedSecret);
10331040
}
10341041

1042+
ForceZero(dh, hpke->Ndh);
1043+
ForceZero(kemContext, hpke->Npk * 2);
10351044
WC_FREE_VAR_EX(dh, hpke->heap, DYNAMIC_TYPE_TMP_BUFFER);
10361045
WC_FREE_VAR_EX(kemContext, hpke->heap, DYNAMIC_TYPE_TMP_BUFFER);
10371046

@@ -1058,6 +1067,7 @@ static int wc_HpkeSetupBaseReceiver(Hpke* hpke, HpkeBaseContext* context,
10581067
infoSz);
10591068
}
10601069

1070+
ForceZero(sharedSecret, hpke->Nsecret);
10611071
WC_FREE_VAR_EX(sharedSecret, hpke->heap, DYNAMIC_TYPE_TMP_BUFFER);
10621072

10631073
return ret;
@@ -1144,6 +1154,7 @@ int wc_HpkeOpenBase(Hpke* hpke, void* receiverKey, const byte* pubKey,
11441154

11451155
PRIVATE_KEY_LOCK();
11461156

1157+
ForceZero(context, sizeof(HpkeBaseContext));
11471158
WC_FREE_VAR_EX(context, hpke->heap, DYNAMIC_TYPE_TMP_BUFFER);
11481159

11491160
return ret;

‎wolfcrypt/src/pkcs12.c‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -637,7 +637,13 @@ static int wc_PKCS12_verify(WC_PKCS12* pkcs12, byte* data, word32 dataSz,
637637
}
638638
#endif
639639

640-
return XMEMCMP(digest, mac->digest, mac->digestSz);
640+
if (ConstantCompare(digest, mac->digest, (int)mac->digestSz) != 0) {
641+
ForceZero(digest, sizeof(digest));
642+
return MAC_CMP_FAILED_E;
643+
}
644+
645+
ForceZero(digest, sizeof(digest));
646+
return 0;
641647
}
642648

643649
int wc_PKCS12_verify_ex(WC_PKCS12* pkcs12, const byte* psw, word32 pswSz)

‎wolfcrypt/src/pwdbased.c‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,8 @@ int wc_PBKDF1_ex(byte* key, int keyLen, byte* iv, int ivLen,
152152

153153
WC_FREE_VAR_EX(hash, heap, DYNAMIC_TYPE_HASHCTX);
154154

155+
ForceZero(digest, sizeof(digest));
156+
155157
if (err != 0)
156158
return err;
157159

@@ -294,6 +296,7 @@ int wc_PBKDF2_ex(byte* output, const byte* passwd, int pLen, const byte* salt,
294296
wc_HmacFree(hmac);
295297
}
296298

299+
ForceZero(buffer, (word32)hLen);
297300
WC_FREE_VAR_EX(buffer, heap, DYNAMIC_TYPE_TMP_BUFFER);
298301
WC_FREE_VAR_EX(hmac, heap, DYNAMIC_TYPE_HMAC);
299302

‎wolfcrypt/src/sakke.c‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6941,7 +6941,8 @@ int wc_DeriveSakkeSSV(SakkeKey* key, enum wc_HashType hashType, byte* ssv,
69416941

69426942
err = sakke_compute_point_r(key, key->id, key->idSz, ri, n, test);
69436943
}
6944-
if ((err == 0) && (XMEMCMP(auth, test, (size_t)(2 * n + 1)) != 0)) {
6944+
/* n is word16, so 2*n+1 always fits in int */
6945+
if ((err == 0) && (ConstantCompare(auth, test, (int)(2 * n + 1)) != 0)) {
69456946
err = SAKKE_VERIFY_FAIL_E;
69466947
}
69476948

‎wolfcrypt/src/srp.c‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -982,7 +982,7 @@ int wc_SrpVerifyPeersProof(Srp* srp, byte* proof, word32 size)
982982
if (hashSize < 0)
983983
return ALGO_ID_E;
984984

985-
if (size != (word32)hashSize)
985+
if (size != (word32)hashSize || size > INT_MAX)
986986
return BUFFER_E;
987987

988988
r = SrpHashFinal(srp->side == SRP_CLIENT_SIDE ? &srp->server_proof
@@ -994,9 +994,11 @@ int wc_SrpVerifyPeersProof(Srp* srp, byte* proof, word32 size)
994994
if (!r) r = SrpHashUpdate(&srp->server_proof, srp->key, srp->keySz);
995995
}
996996

997-
if (!r && XMEMCMP(proof, digest, size) != 0)
997+
if (!r && ConstantCompare(proof, digest, (int)size) != 0)
998998
r = SRP_VERIFY_E;
999999

1000+
ForceZero(digest, sizeof(digest));
1001+
10001002
return r;
10011003
}
10021004

0 commit comments

Comments
 (0)