Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions tests/api/test_aes.c
Original file line number Diff line number Diff line change
Expand Up @@ -3233,13 +3233,13 @@ int test_wc_AesGcmEncryptDecrypt_Sizes(void)
int sz;
int i;
WC_DECLARE_VAR(plain, byte, GCM_LEN, NULL);
WC_DECLARE_VAR(cipher, byte, GCM_LEN, NULL);
WC_DECLARE_VAR(cipher, byte, GCM_LEN+WC_AES_BLOCK_SIZE, NULL);
#ifdef HAVE_AES_DECRYPT
WC_DECLARE_VAR(decrypted, byte, GCM_LEN, NULL);
#endif

WC_ALLOC_VAR(plain, byte, GCM_LEN, NULL);
WC_ALLOC_VAR(cipher, byte, GCM_LEN, NULL);
WC_ALLOC_VAR(cipher, byte, GCM_LEN+WC_AES_BLOCK_SIZE, NULL);
#ifdef HAVE_AES_DECRYPT
WC_ALLOC_VAR(decrypted, byte, GCM_LEN, NULL);
#endif
Expand Down Expand Up @@ -3964,7 +3964,8 @@ int test_wc_AesGcmNonStdNonce(void)
* and cannot exercise the GHASH-based counter derivation. */
#if !defined(NO_AES) && defined(HAVE_AESGCM) && \
!defined(HAVE_FIPS) && \
!defined(WOLFSSL_AFALG) && !defined(WOLFSSL_KCAPI)
!defined(WOLFSSL_AFALG) && !defined(WOLFSSL_KCAPI) && \
!defined(WOLFSSL_DEVCRYPTO_AES)

/* ------------------------------------------------------------------
* Section 1: 1-byte IV, AES-128
Expand Down
4 changes: 2 additions & 2 deletions wolfcrypt/src/aes.c
Original file line number Diff line number Diff line change
Expand Up @@ -5837,7 +5837,7 @@ static void AesSetKey_C(Aes* aes, const byte* key, word32 keySz, int dir)

#if defined(WOLFSSL_DEVCRYPTO) && \
(defined(WOLFSSL_DEVCRYPTO_AES) || defined(WOLFSSL_DEVCRYPTO_CBC))
aes->ctx.cfd = -1;
aes->ctx.inited = 0;
#endif
#ifdef WOLFSSL_IMX6_CAAM_BLOB
ForceZero(local, sizeof(local));
Expand Down Expand Up @@ -14885,7 +14885,7 @@ int wc_AesInit(Aes* aes, void* heap, int devId)
#endif
#if defined(WOLFSSL_DEVCRYPTO) && \
(defined(WOLFSSL_DEVCRYPTO_AES) || defined(WOLFSSL_DEVCRYPTO_CBC))
aes->ctx.cfd = -1;
aes->ctx.inited = 0;
#endif
#if defined(WOLFSSL_IMXRT_DCP)
DCPAesInit(aes);
Expand Down
12 changes: 8 additions & 4 deletions wolfcrypt/src/des3.c
Original file line number Diff line number Diff line change
Expand Up @@ -1571,10 +1571,14 @@

/* rotate left and right halves independently */
for (j = 0; j < 48; j++) { /* select bits individually */
if (pcr[pc2[j] - 1]) { /* check bit that goes to ks[j] */
l= j % 6; /* mask it in if it's there */
ks[j/6] |= (byte)(bytebit[l] >> 2);
}
byte bit;
byte mask;
bit =
(byte)(pcr[pc2[j] - 1]); /* all pcr values are either 0 or 1 */
mask = (byte)(0 - bit); /* mask is either 0xFF or 0x00 */
/* only set to bytebit value if bit == 1*/
ks[j/6] |=
(byte)((bytebit[j % 6] >> 2) & mask);
}

/* Now convert to odd/even interleaved form for use in F */
Expand Down
7 changes: 7 additions & 0 deletions wolfcrypt/src/dsa.c
Original file line number Diff line number Diff line change
Expand Up @@ -1124,6 +1124,13 @@ int wc_DsaVerify_ex(const byte* digest, word32 digestSz, const byte* sig,
if (digest == NULL || sig == NULL || key == NULL || answer == NULL)
return BAD_FUNC_ARG;

/* assign default value so we return 0 on error */
*answer = 0;

/* Note the min allowed digestSz here is WC_SHA_DIGEST_SIZE, not
* WC_MIN_DIGEST_SIZE, to allow verify-only legacy DSA operations, as
* expressly allowed under FIPS 186-5, FIPS 140-3, and SP 800-131A.
*/
if ((digestSz > WC_MAX_DIGEST_SIZE) ||
(digestSz < WC_MIN_DIGEST_SIZE_FOR_VERIFY))
{
Expand Down
10 changes: 3 additions & 7 deletions wolfcrypt/src/ecc.c
Original file line number Diff line number Diff line change
Expand Up @@ -256,10 +256,6 @@ ECC Curve Sizes:
#include <wolfssl/wolfcrypt/port/cypress/psoc6_crypto.h>
#endif

#if defined(WOLFSSL_CAAM)
#include <wolfssl/wolfcrypt/port/caam/wolfcaam.h>
#endif

#if defined(WOLFSSL_KCAPI_ECC)
#include <wolfssl/wolfcrypt/port/kcapi/kcapi_ecc.h>
#endif
Expand Down Expand Up @@ -10111,7 +10107,7 @@ static int _ecc_export_x963(ecc_key* key, byte* out, word32* outLen)
/* store byte point type */
out[0] = ECC_POINT_UNCOMP;

if (caamReadPartition((CAAM_ADDRESS)key->securePubKey, out+1, keySz*2) != 0)
if (caamReadPartition(key->securePubKey, out+1, keySz*2) != 0)
return WC_HW_E;

*outLen = 1 + 2*keySz;
Expand Down Expand Up @@ -11734,15 +11730,15 @@ static int _ecc_import_private_key_ex(const byte* priv, word32 privSz,
}

key->partNum = part;
key->blackKey = (word32)vaddr;
key->blackKey = vaddr;
if (caamWriteToPartition(vaddr, priv, privSz) != 0)
return WC_HW_E;

if (pub != NULL) {
/* +1 to account for x963 compressed bit */
if (caamWriteToPartition(vaddr + privSz, pub + 1, pubSz - 1) != 0)
return WC_HW_E;
key->securePubKey = (word32)vaddr + privSz;
key->securePubKey = vaddr + privSz;
}
}
else {
Expand Down
2 changes: 1 addition & 1 deletion wolfcrypt/src/hmac.c
Original file line number Diff line number Diff line change
Expand Up @@ -1519,7 +1519,7 @@ int wc_HmacInit(Hmac* hmac, void* heap, int devId)
hmac->devCtx = NULL;
#endif
#if defined(WOLFSSL_DEVCRYPTO_HMAC)
hmac->ctx.cfd = -1;
hmac->ctx.inited = 0;
#endif

#if defined(WOLFSSL_ASYNC_CRYPT) && defined(WC_ASYNC_ENABLE_HMAC)
Expand Down
8 changes: 3 additions & 5 deletions wolfcrypt/src/port/Espressif/esp_crt_bundle/esp_crt_bundle.c
Original file line number Diff line number Diff line change
Expand Up @@ -983,14 +983,12 @@ static CB_INLINE int wolfssl_ssl_conf_verify_cb_no_signer(int preverify,
/* Clean up and exit */
if ((_crt_found == 0) && (bundle_cert != NULL)) {
ESP_LOGW(TAG, "Cert not found, free bundle_cert");
/* this_subject and this_issuer are apart of bundle_cert and will be
* freed here*/
wolfSSL_X509_free(bundle_cert);
bundle_cert = NULL;
/* this_subject and this_issuer are pointers into cert used.
* Don't free if the cert was found. */
wolfSSL_X509_NAME_free(this_subject);
this_subject = NULL;
wolfSSL_X509_NAME_free(this_issuer);
this_issuer = NULL;
this_subject = NULL;
}

/* We don't clean up the store_cert and x509 as we are in a callback,
Expand Down
20 changes: 13 additions & 7 deletions wolfcrypt/src/port/Renesas/renesas_fspsm_aes.c
Original file line number Diff line number Diff line change
Expand Up @@ -410,10 +410,13 @@ int wc_fspsm_AesGcmEncrypt(struct Aes* aes, byte* out,
aes->heap, DYNAMIC_TYPE_AES);
key_server_aes = (FSPSM_AES_PWKEY)XMALLOC(sizeof(FSPSM_AES_WKEY),
aes->heap, DYNAMIC_TYPE_AES);
if (key_client_aes == NULL || key_server_aes == NULL) {
XFREE(plainBuf, aes->heap, DYNAMIC_TYPE_AES);
XFREE(cipherBuf, aes->heap, DYNAMIC_TYPE_AES);
XFREE(aTagBuf, aes->heap, DYNAMIC_TYPE_AES);
if (key_server_aes == NULL || key_client_aes == NULL) {
XFREE(key_client_aes, aes->heap, DYNAMIC_TYPE_AES);
XFREE(key_server_aes, aes->heap, DYNAMIC_TYPE_AES);
XFREE(plainBuf, aes->heap, DYNAMIC_TYPE_AES);
XFREE(cipherBuf, aes->heap, DYNAMIC_TYPE_AES);
XFREE(aTagBuf, aes->heap, DYNAMIC_TYPE_AES);
wc_fspsm_hw_unlock();
return MEMORY_E;
}

Expand Down Expand Up @@ -638,9 +641,12 @@ int wc_fspsm_AesGcmDecrypt(struct Aes* aes, byte* out,
key_server_aes = (FSPSM_AES_PWKEY)XMALLOC(sizeof(FSPSM_AES_WKEY),
aes->heap, DYNAMIC_TYPE_AES);
if (key_client_aes == NULL || key_server_aes == NULL) {
XFREE(plainBuf, aes->heap, DYNAMIC_TYPE_AES);
XFREE(cipherBuf, aes->heap, DYNAMIC_TYPE_AES);
XFREE(aTagBuf, aes->heap, DYNAMIC_TYPE_AES);
XFREE(key_client_aes, aes->heap, DYNAMIC_TYPE_AES);
XFREE(key_server_aes, aes->heap, DYNAMIC_TYPE_AES);
XFREE(plainBuf, aes->heap, DYNAMIC_TYPE_AES);
XFREE(cipherBuf, aes->heap, DYNAMIC_TYPE_AES);
XFREE(aTagBuf, aes->heap, DYNAMIC_TYPE_AES);
wc_fspsm_hw_unlock();
return MEMORY_E;
}

Expand Down
2 changes: 1 addition & 1 deletion wolfcrypt/src/port/Renesas/renesas_fspsm_sha.c
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ static int FSPSM_HashFinal(wolfssl_FSPSM_Hash* hash, byte* out, word32 outSz)
#endif
wc_fspsm_hw_lock();

if (Init(&handle) == FSP_SUCCESS) {
if ((ret = Init(&handle)) == FSP_SUCCESS) {
ret = Update(&handle, (uint8_t*)hash->msg, hash->used);
if (ret == FSP_SUCCESS) {
ret = Final(&handle, out, (uint32_t*)&sz);
Expand Down
16 changes: 7 additions & 9 deletions wolfcrypt/src/port/devcrypto/devcrypto_aes.c
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ int wc_AesCbcEncrypt(Aes* aes, byte* out, const byte* in, word32 sz)
if (sz == 0) {
return 0;
}
if (aes->ctx.cfd == -1) {
if (aes->ctx.inited == 0) {
ret = wc_DevCryptoCreate(&aes->ctx, CRYPTO_AES_CBC,
(byte*)aes->devKey, aes->keylen);
if (ret != 0)
Expand Down Expand Up @@ -82,7 +82,7 @@ int wc_AesCbcDecrypt(Aes* aes, byte* out, const byte* in, word32 sz)
}

XMEMCPY(aes->tmp, in + sz - WC_AES_BLOCK_SIZE, WC_AES_BLOCK_SIZE);
if (aes->ctx.cfd == -1) {
if (aes->ctx.inited == 0) {
ret = wc_DevCryptoCreate(&aes->ctx, CRYPTO_AES_CBC,
(byte*)aes->devKey, aes->keylen);
if (ret != 0)
Expand Down Expand Up @@ -110,7 +110,7 @@ int wc_AesSetKey(Aes* aes, const byte* userKey, word32 keylen,
const word32 max_key_len = (AES_MAX_KEY_SIZE / 8);
#endif

if (aes == NULL ||
if (aes == NULL || userKey == NULL ||
!((keylen == 16) || (keylen == 24) || (keylen == 32))) {
return BAD_FUNC_ARG;
}
Expand All @@ -129,6 +129,7 @@ int wc_AesSetKey(Aes* aes, const byte* userKey, word32 keylen,
aes->left = 0;
#endif
aes->ctx.cfd = -1;
aes->ctx.inited = 0;
XMEMCPY(aes->devKey, userKey, keylen);

(void)dir;
Expand All @@ -151,7 +152,7 @@ static int wc_DevCrypto_AesDirect(Aes* aes, byte* out, const byte* in,
return BAD_FUNC_ARG;
}

if (aes->ctx.cfd == -1) {
if (aes->ctx.inited == 0) {
ret = wc_DevCryptoCreate(&aes->ctx, CRYPTO_AES_ECB, (byte*)aes->devKey,
aes->keylen);
if (ret != 0)
Expand Down Expand Up @@ -221,7 +222,7 @@ int wc_AesCtrEncrypt(Aes* aes, byte* out, const byte* in, word32 sz)
sz--;
}

if (aes->ctx.cfd == -1) {
if (aes->ctx.inited == 0) {
ret = wc_DevCryptoCreate(&aes->ctx, CRYPTO_AES_CTR, (byte*)aes->devKey,
aes->keylen);
if (ret != 0)
Expand Down Expand Up @@ -284,8 +285,6 @@ int wc_AesGcmSetKey(Aes* aes, const byte* key, word32 len)
return wc_AesSetKey(aes, key, len, NULL, AES_ENCRYPTION);
}



/* common code for AES-GCM encrypt/decrypt */
static int wc_DevCrypto_AesGcm(Aes* aes, byte* out, byte* in, word32 sz,
const byte* iv, word32 ivSz,
Expand All @@ -310,7 +309,7 @@ static int wc_DevCrypto_AesGcm(Aes* aes, byte* out, byte* in, word32 sz,
in = scratch;

XMEMSET(scratch, 0, WC_AES_BLOCK_SIZE);
if (aes->ctx.cfd == -1) {
if (aes->ctx.inited == 0) {
ret = wc_DevCryptoCreate(&aes->ctx, CRYPTO_AES_GCM, (byte*)aes->devKey,
aes->keylen);
if (ret != 0)
Expand Down Expand Up @@ -399,4 +398,3 @@ int wc_AesEcbDecrypt(Aes* aes, byte* out, const byte* in, word32 sz)
#endif /* HAVE_AES_ECB */
#endif /* WOLFSSL_DEVCRYPTO_AES */
#endif /* !NO_AES && WOLFSSL_DEVCRYPTO */

51 changes: 33 additions & 18 deletions wolfcrypt/src/port/devcrypto/devcrypto_hash.c
Original file line number Diff line number Diff line change
Expand Up @@ -135,20 +135,14 @@ int wc_Sha256Update(wc_Sha256* sha, const byte* in, word32 sz)
#ifdef WOLFSSL_DEVCRYPTO_HASH_KEEP
/* keep full message to hash at end instead of incremental updates */
if (sha->len < sha->used + sz) {
if (sha->msg == NULL) {
sha->msg = (byte*)XMALLOC(sha->used + sz, sha->heap,
DYNAMIC_TYPE_TMP_BUFFER);
} else {
byte* pt = (byte*)XREALLOC(sha->msg, sha->used + sz, sha->heap,
DYNAMIC_TYPE_TMP_BUFFER);
if (pt == NULL) {
return MEMORY_E;
}
sha->msg = pt;
}
if (sha->msg == NULL) {
byte* pt = (byte*)XREALLOC(sha->msg, sha->used + sz, sha->heap,
DYNAMIC_TYPE_TMP_BUFFER);
if (pt == NULL) {
return MEMORY_E;
}

sha->msg = pt;

sha->len = sha->used + sz;
}
XMEMCPY(sha->msg + sha->used, in, sz);
Expand All @@ -173,14 +167,16 @@ int wc_Sha256Final(wc_Sha256* sha, byte* hash)
#ifdef WOLFSSL_DEVCRYPTO_HASH_KEEP
/* keep full message to hash at end instead of incremental updates */
if ((ret = HashUpdate(sha, CRYPTO_SHA2_256, sha->msg, sha->used)) < 0) {
wc_Sha256Free(sha);
return ret;
}
XFREE(sha->msg, sha->heap, DYNAMIC_TYPE_TMP_BUFFER);
sha->msg = NULL;
#endif
ret = GetDigest(sha, CRYPTO_SHA2_256, hash);
if (ret != 0) {
return ret;
wc_Sha256Free(sha);
return ret;
}

wc_Sha256Free(sha);
Expand All @@ -198,9 +194,11 @@ int wc_Sha256GetHash(wc_Sha256* sha, byte* hash)
{
int ret;
wc_Sha256 cpy;
wc_Sha256Copy(sha, &cpy);
XMEMSET(&cpy, 0, sizeof(cpy)); /* ZII */
ret = wc_Sha256Copy(sha, &cpy);

if ((ret = HashUpdate(&cpy, CRYPTO_SHA2_256, cpy.msg, cpy.used)) == 0) {
if (ret == 0 &&
(ret = HashUpdate(&cpy, CRYPTO_SHA2_256, cpy.msg, cpy.used)) == 0) {
/* help static analysis tools out */
XMEMSET(hash, 0, WC_SHA256_DIGEST_SIZE);
ret = GetDigest(&cpy, CRYPTO_SHA2_256, hash);
Expand All @@ -219,22 +217,39 @@ int wc_Sha256GetHash(wc_Sha256* sha, byte* hash)

int wc_Sha256Copy(wc_Sha256* src, wc_Sha256* dst)
{
int ret = 0;

if (src == NULL || dst == NULL) {
return BAD_FUNC_ARG;
}

wc_InitSha256_ex(dst, src->heap, 0);
#ifdef WOLFSSL_DEVCRYPTO_HASH_KEEP

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [Medium] devcrypto wc_Sha256Copy frees the destination before initializing it, reading an uninitialized dst (arbitrary fd close… · Security

BEFORE, devcrypto wc_Sha256Copy called wc_InitSha256_ex(dst, ...) FIRST, which does XMEMSET(sha, 0, sizeof(wc_Sha256)) and is therefore safe on an uninitialized/garbage dst. AFTER, the function calls wc_Sha256Free(dst) as its very first action (to fix a leak of a pre-existing dst session/msg). For WOLFSSL_DEVCRYPTO_HASH, wc_Sha256Free executes wc_DevCryptoFree(&dst->ctx) -> if the garbage dst->ctx.inited bit reads as 1 it runs ioctl(dst->ctx.cfd, ...) and close(dst->ctx.cfd) on a garbage descriptor (closing an unrelated live fd/socket in the process), and for WOLFSSL_DEVCRYPTO_HASH_KEEP it runs ForceZero(dst->msg, dst->len) then XFREE(dst->msg) on a garbage pointer/length (arbitrary write + invalid free / heap corruption). This changes the long-standing wc_XxxCopy contract ("the copy methods overwrite the entire dest low level hash struct"), which the generic sha256.c backend still honors via XMEMCPY(dst, src, sizeof) without touching dst first. wc_Sha256Copy is a public WOLFSSL_API. The one internal caller that could hit this (wc_Sha256GetHash) was patched in the same diff with XMEMSET(&cpy, 0, sizeof(cpy)), and the in-tree SHA-256 call sites zero dst first, so in-tree exploitability is low; but any external consumer that relied on the previous "copy overwrites everything" behavior and passed an uninitialized dst now triggers undefined behavior.

Fix: Zero/initialize dst before freeing it, or free the prior contents conditionally. Simplest: XMEMSET(dst, 0, sizeof(*dst)); (or restore the original order: init dst first) and only free a caller-supplied dst when the API contract explicitly guarantees dst is already initialized. Alternatively, document and enforce that dst must be a zeroed/initialized object and update the sha256.h contract for all backends.

wc_Sha256Free(dst);
if ((ret = wc_InitSha256_ex(dst, src->heap, 0)) != 0) {
/* make sure that any attempts to free dst
* dont accidentally close an unopened fd */
dst->ctx.inited = 0;
dst->ctx.cfd = -1;
return ret;
}
dst->len = src->len;
dst->used = src->used;
dst->msg = (byte*)XMALLOC(src->len, dst->heap, DYNAMIC_TYPE_TMP_BUFFER);
if (dst->msg == NULL) {
wc_Sha256Free(dst);
return MEMORY_E;
}
XMEMCPY(dst->msg, src->msg, src->len);
#endif

return 0;
return ret;
#else

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [Medium] wc_Sha256Copy now returns NOT_COMPILED_IN without WOLFSSL_DEVCRYPTO_HASH_KEEP · api

When WOLFSSL_DEVCRYPTO_HASH_KEEP is not defined, wc_Sha256Copy now returns NOT_COMPILED_IN instead of silently succeeding. This is the intended fix (the previous copy produced a non-functional/empty context), and failing loudly is correct. However, it is a behavior change for a public API: any consumer that copies a running SHA-256 state in a devcrypto-hash build without HASH_KEEP (e.g. TLS transcript-hash duplication, PRF, wc_HashCopy) will now hard-fail rather than silently misbehave. This should be intentional and documented so integrators enable WOLFSSL_DEVCRYPTO_HASH_KEEP when copy-dependent operations (TLS) are used with devcrypto hashing.

Fix: Confirm this is intended and note in the devcrypto documentation/config that WOLFSSL_DEVCRYPTO_HASH_KEEP is required whenever hash-copy (e.g. TLS) is needed; consider a build-time guard/warning to catch the misconfiguration.

(void)src;
(void)dst;
(void)ret;

WOLFSSL_MSG("Compile with WOLFSSL_DEVCRYPTO_HASH_KEEP for this feature");
return NOT_COMPILED_IN;
#endif
}

#endif /* !NO_SHA256 */
Expand Down
1 change: 1 addition & 0 deletions wolfcrypt/src/port/devcrypto/devcrypto_hmac.c
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ int wc_DevCrypto_HmacSetKey(Hmac* hmac, int t, const byte* key, word32 keySz)
{
int hType;

hmac->ctx.inited = 0;
hmac->ctx.cfd = -1;
hType = InternalTypeToDevcrypto(t);
if (hType < 0) {
Expand Down
2 changes: 0 additions & 2 deletions wolfcrypt/src/port/devcrypto/devcrypto_rsa.c
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,6 @@ static int _PublicOperation(const byte* in, word32 inlen, byte* out,

dev = &key->ctx;

key->ctx.cfd = -1;
if (wc_DevCryptoCreate(dev, CRYPTO_ASYM_RSA_PUBLIC, NULL, 0) != 0) {
WOLFSSL_MSG("Error getting RSA public session");
return WC_DEVCRYPTO_E;
Expand Down Expand Up @@ -439,7 +438,6 @@ int wc_DevCrypto_MakeRsaKey(RsaKey* key, int size, long e, WC_RNG* rng)
byte eBuf[8];
int eBufSz;

key->ctx.cfd = -1;
nSz = dSz = bSz;
cSz = pSz = qSz = dpSz = dqSz = bSz/2;

Expand Down
Loading
Loading