Skip to content

20260616-aes-fixes #10709

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
douzzer wants to merge 4 commits into
base: master
Choose a base branch
from
Open

20260616-aes-fixes #10709

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5def276
wolfcrypt/src/aes.c: catch and error on total length overflow in wc_A…
douzzer Jun 16, 2026
1070384
wolfcrypt/src/aes.c and wolfcrypt/test/test.c: fixes from review, re …
douzzer Jun 17, 2026
9d15bc7
wolfcrypt/src/aes.c, wolfcrypt/src/port/caam/caam_aes.c, wolfcrypt/sr…
douzzer Jun 17, 2026
881fe76
wolfcrypt/src/aes.c, wolfcrypt/test/test.c: use WOLFSSL_MAX_32BIT rat…
douzzer Jun 17, 2026
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
73 changes: 71 additions & 2 deletions wolfcrypt/src/aes.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12808,6 +12808,18 @@ int wc_AesGcmEncryptUpdate(Aes* aes, byte* out, const byte* in, word32 sz,
ret = MISSING_IV;
}

/* Prevent overflow of aes->cSz and ->aSz. Per NIST SP 800-38D section
* 5.2.1.1, the maximum allowed ciphertext limit is 2^32 - 2 blocks, but we
Comment thread
Frauschi marked this conversation as resolved.
* currently pass around the cumulative sizes in bytes as word32s, so we
* can't currently support the maximum allowed.
*/
if ((ret == 0) &&
Comment thread
Frauschi marked this conversation as resolved.
((aes->cSz > WOLFSSL_MAX_32BIT - sz) ||
(aes->aSz > WOLFSSL_MAX_32BIT - authInSz)))
{
ret = AES_GCM_OVERFLOW_E;
}

if ((ret == 0) && aes->ctrSet && (aes->aSz == 0) && (aes->cSz == 0)) {
aes->invokeCtr[0]++;
if (aes->invokeCtr[0] == 0) {
Expand Down Expand Up @@ -12950,6 +12962,18 @@ int wc_AesGcmDecryptUpdate(Aes* aes, byte* out, const byte* in, word32 sz,
ret = MISSING_IV;
}

/* Prevent overflow of aes->cSz and ->aSz. Per NIST SP 800-38D section
* 5.2.1.1, the maximum allowed ciphertext limit is 2^32 - 2 blocks, but we
* currently pass around the cumulative sizes in bytes as word32s, so we
* can't currently support the maximum allowed.
*/
if ((ret == 0) &&
((aes->cSz > WOLFSSL_MAX_32BIT - sz) ||
(aes->aSz > WOLFSSL_MAX_32BIT - authInSz)))
{
ret = AES_GCM_OVERFLOW_E;
}

if (ret == 0) {
/* Decrypt with AAD and/or cipher text. */
#ifdef WOLFSSL_AESNI
Expand Down Expand Up @@ -13360,6 +13384,18 @@ int wc_AesCcmEncrypt(Aes* aes, byte* out, const byte* in, word32 inSz,
return status;
}

{
word32 lenSz = (word32)WC_AES_BLOCK_SIZE - 1U - nonceSz;
/* With a large nonce, B[] runs out of room to represent inSz, and beyond
* that, the counter itself can wrap.
*/
if ((lenSz < sizeof(inSz)) &&
(inSz >= ((word32)1 << (lenSz * 8))))
{
return AES_CCM_OVERFLOW_E;
}
}

status = wolfSSL_CryptHwMutexLock();
if (status != 0)
return status;
Expand Down Expand Up @@ -13394,6 +13430,18 @@ int wc_AesCcmDecrypt(Aes* aes, byte* out, const byte* in, word32 inSz,
return status;
}

{
word32 lenSz = (word32)WC_AES_BLOCK_SIZE - 1U - nonceSz;
/* With a large nonce, B[] runs out of room to represent inSz, and beyond
* that, the counter itself can wrap.
*/
if ((lenSz < sizeof(inSz)) &&
(inSz >= ((word32)1 << (lenSz * 8))))
{
return AES_CCM_OVERFLOW_E;
}
}

status = wolfSSL_CryptHwMutexLock();
if (status != 0)
return status;
Expand Down Expand Up @@ -13586,6 +13634,17 @@ int wc_AesCcmEncrypt(Aes* aes, byte* out, const byte* in, word32 inSz,
return BAD_FUNC_ARG;
}

lenSz = (byte)(WC_AES_BLOCK_SIZE - 1U - nonceSz);

/* With a large nonce, B[] runs out of room to represent inSz, and beyond
* that, the counter itself can wrap.
*/
if ((lenSz < sizeof(inSz)) &&

@dgarske dgarske Jun 17, 2026

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.

🔵 [Low] Reused AES_CCM_OVERFLOW_E error string is about 'invocation counter', not input length

The new CCM checks return AES_CCM_OVERFLOW_E, whose human-readable string (error.c:503-504) and header comment (error-crypt.h:255) describe it as 'AES-CCM invocation counter overflow'. The new condition is actually an input-length-vs-nonce-size limitation (the message length doesn't fit the CCM L field), not an invocation-counter overflow. Reusing the closest existing code is reasonable and avoids adding a new error number, but the diagnostic string may mislead someone debugging the failure. (The GCM reuse of AES_GCM_OVERFLOW_E for cumulative-size overflow is a closer semantic fit.)

Fix: Consider broadening the error string (e.g. 'AES-CCM length/counter overflow') so it covers the input-size case, or document the dual meaning. Not blocking.

@douzzer douzzer Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The error string does need updating. Have made a note and will fix in a follow-up PR, or if this one needs another spin for a substantive reason, will fix it here.

@douzzer douzzer Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed.

@dgarske dgarske Jun 17, 2026

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.

🔵 [Low] Raw sizeof in mixed-signedness comparison deviates from function's wordSz convention · convention

The new check (lenSz < sizeof(inSz)) compares lenSz (a byte, integer-promoted to signed int) against sizeof(inSz) (size_t, unsigned). Both functions already define const word32 wordSz = (word32)sizeof(word32); precisely to avoid comparing against a raw sizeof (used in the loop a few lines below). Using the raw sizeof here is inconsistent with that local convention; depending on warning flags it can also surface as a signed/unsigned comparison note. The logic itself is correct (lenSz is small and non-negative).

Fix: Use the existing wordSz (with an explicit cast on lenSz) to keep the comparison consistent with the rest of the function and avoid mixed-signedness: if (((word32)lenSz < wordSz) && (inSz >= ((word32)1 << (lenSz * 8)))) { return AES_CCM_OVERFLOW_E; }

@douzzer douzzer Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not something to fix. The same new code pattern is used at 10 different code points, with widely varying local context. wordSz is only available at 2 of them. It is more useful to keep the checks identical, than to fork 2 of the 10 to leverage the (pointless) wordSz stack allocation.

(inSz >= ((word32)1 << (lenSz * 8))))
{
return AES_CCM_OVERFLOW_E;
}

#ifdef WOLF_CRYPTO_CB
#ifndef WOLF_CRYPTO_CB_FIND
if (aes->devId != INVALID_DEVID)
Expand All @@ -13602,7 +13661,7 @@ int wc_AesCcmEncrypt(Aes* aes, byte* out, const byte* in, word32 inSz,

XMEMSET(A, 0, sizeof(A));
XMEMCPY(B+1, nonce, nonceSz);
lenSz = (byte)(WC_AES_BLOCK_SIZE - 1U - nonceSz);

B[0] = (byte)((authInSz > 0 ? 64 : 0)
+ (8 * (((byte)authTagSz - 2) / 2))
+ (lenSz - 1));
Expand Down Expand Up @@ -13739,6 +13798,17 @@ int wc_AesCcmDecrypt(Aes* aes, byte* out, const byte* in, word32 inSz,
return BAD_FUNC_ARG;
}

lenSz = (byte)(WC_AES_BLOCK_SIZE - 1U - nonceSz);

/* With a large nonce, B[] runs out of room to represent inSz, and beyond
* that, the counter itself can wrap.
*/
if ((lenSz < sizeof(inSz)) &&
(inSz >= ((word32)1 << (lenSz * 8))))
{
return AES_CCM_OVERFLOW_E;
}

#ifdef WOLF_CRYPTO_CB
#ifndef WOLF_CRYPTO_CB_FIND
if (aes->devId != INVALID_DEVID)
Expand All @@ -13757,7 +13827,6 @@ int wc_AesCcmDecrypt(Aes* aes, byte* out, const byte* in, word32 inSz,
oSz = inSz;
XMEMSET(A, 0, sizeof A);
XMEMCPY(B+1, nonce, nonceSz);
lenSz = (byte)(WC_AES_BLOCK_SIZE - 1U - nonceSz);

B[0] = (byte)(lenSz - 1U);
for (i = 0; i < lenSz; i++)
Expand Down
4 changes: 2 additions & 2 deletions wolfcrypt/src/error.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -498,10 +498,10 @@ const char* wc_GetErrorString(int error)
return "wolfcrypt FIPS ECDHE Known Answer Test Failure";

case AES_GCM_OVERFLOW_E:
return "AES-GCM invocation counter overflow";
return "AES-GCM internal overflow averted";

case AES_CCM_OVERFLOW_E:
return "AES-CCM invocation counter overflow";
return "AES-CCM internal overflow averted";

case RSA_KEY_PAIR_E:
return "RSA Key Pair-Wise Consistency check fail";
Expand Down
28 changes: 24 additions & 4 deletions wolfcrypt/src/port/caam/caam_aes.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,7 @@ int wc_AesCcmEncrypt(Aes* aes, byte* out,
word32 keySz;
word32 i;
byte B0Ctr0[WC_AES_BLOCK_SIZE + WC_AES_BLOCK_SIZE];
int lenSz;
word32 lenSz;
byte mask = 0xFF;
const word32 wordSz = (word32)sizeof(word32);
int ret;
Expand All @@ -513,10 +513,20 @@ int wc_AesCcmEncrypt(Aes* aes, byte* out,
return ret;
}

lenSz = WC_AES_BLOCK_SIZE - 1 - (byte)nonceSz;
/* With a large nonce, B[] runs out of room to represent inSz, and beyond
* that, the counter itself can wrap.
*/
if ((lenSz < sizeof(inSz)) &&
(inSz >= ((word32)1 << (lenSz * 8))))
{
return AES_CCM_OVERFLOW_E;
}

/* set up B0 and CTR0 similar to how wolfcrypt/src/aes.c does */
XMEMCPY(B0Ctr0+1, nonce, nonceSz);
XMEMCPY(B0Ctr0+WC_AES_BLOCK_SIZE+1, nonce, nonceSz);
lenSz = WC_AES_BLOCK_SIZE - 1 - (byte)nonceSz;

B0Ctr0[0] = (authInSz > 0 ? 64 : 0)
+ (8 * (((byte)authTagSz - 2) / 2))
+ (lenSz - 1);
Expand Down Expand Up @@ -577,7 +587,7 @@ int wc_AesCcmDecrypt(Aes* aes, byte* out,
word32 i;
byte B0Ctr0[WC_AES_BLOCK_SIZE + WC_AES_BLOCK_SIZE];
byte tag[WC_AES_BLOCK_SIZE];
int lenSz;
word32 lenSz;
byte mask = 0xFF;
const word32 wordSz = (word32)sizeof(word32);
int ret;
Expand All @@ -596,10 +606,20 @@ int wc_AesCcmDecrypt(Aes* aes, byte* out,
return ret;
}

lenSz = WC_AES_BLOCK_SIZE - 1 - (byte)nonceSz;
/* With a large nonce, B[] runs out of room to represent inSz, and beyond
* that, the counter itself can wrap.
*/
if ((lenSz < sizeof(inSz)) &&
(inSz >= ((word32)1 << (lenSz * 8))))
{
return AES_CCM_OVERFLOW_E;
}

/* set up B0 and CTR0 similar to how wolfcrypt/src/aes.c does */
XMEMCPY(B0Ctr0+1, nonce, nonceSz);
XMEMCPY(B0Ctr0+WC_AES_BLOCK_SIZE+1, nonce, nonceSz);
lenSz = WC_AES_BLOCK_SIZE - 1 - (byte)nonceSz;

B0Ctr0[0] = (authInSz > 0 ? 64 : 0)
+ (8 * (((byte)authTagSz - 2) / 2))
+ (lenSz - 1);
Expand Down
33 changes: 29 additions & 4 deletions wolfcrypt/src/port/riscv/riscv-64-aes.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9102,6 +9102,7 @@ int wc_AesCcmEncrypt(Aes* aes, byte* out, const byte* in, word32 inSz,
const byte* authIn, word32 authInSz)
{
int ret = 0;
byte lenSz = 0;

/* sanity check on arguments */
if ((aes == NULL) || ((inSz != 0) && ((in == NULL) || (out == NULL))) ||
Expand All @@ -9114,14 +9115,26 @@ int wc_AesCcmEncrypt(Aes* aes, byte* out, const byte* in, word32 inSz,
ret = BAD_FUNC_ARG;
}

if (ret == 0) {
lenSz = WC_AES_BLOCK_SIZE - 1 - (byte)nonceSz;

/* With a large nonce, B[] runs out of room to represent inSz, and beyond
* that, the counter itself can wrap.
*/
if ((lenSz < sizeof(inSz)) &&
(inSz >= ((word32)1 << (lenSz * 8))))
{
ret = AES_CCM_OVERFLOW_E;
}
}

if (ret == 0) {
byte A[WC_AES_BLOCK_SIZE];
byte B[WC_AES_BLOCK_SIZE];
byte lenSz;
byte i;

XMEMCPY(B+1, nonce, nonceSz);
lenSz = WC_AES_BLOCK_SIZE - 1 - (byte)nonceSz;

B[0] = (authInSz > 0 ? 64 : 0)
+ (8 * (((byte)authTagSz - 2) / 2))
+ (lenSz - 1);
Expand Down Expand Up @@ -9180,6 +9193,7 @@ int wc_AesCcmDecrypt(Aes* aes, byte* out, const byte* in, word32 inSz,
const byte* authIn, word32 authInSz)
{
int ret = 0;
byte lenSz = 0;

/* sanity check on arguments */
if ((aes == NULL) || ((inSz != 0) && ((in == NULL) || (out == NULL))) ||
Expand All @@ -9192,16 +9206,27 @@ int wc_AesCcmDecrypt(Aes* aes, byte* out, const byte* in, word32 inSz,
ret = BAD_FUNC_ARG;
}

if (ret == 0) {
lenSz = WC_AES_BLOCK_SIZE - 1 - (byte)nonceSz;

/* With a large nonce, B[] runs out of room to represent inSz, and beyond
* that, the counter itself can wrap.
*/
if ((lenSz < sizeof(inSz)) &&
(inSz >= ((word32)1 << (lenSz * 8))))
{
ret = AES_CCM_OVERFLOW_E;
}
}

if (ret == 0) {
byte A[WC_AES_BLOCK_SIZE];
byte B[WC_AES_BLOCK_SIZE];
byte lenSz;
byte i;
byte* o = out;
word32 oSz = inSz;

XMEMCPY(B+1, nonce, nonceSz);
lenSz = WC_AES_BLOCK_SIZE - 1 - (byte)nonceSz;

B[0] = lenSz - 1;
for (i = 0; i < lenSz; i++) {
Expand Down
26 changes: 26 additions & 0 deletions wolfcrypt/src/port/silabs/silabs_aes.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,10 +273,23 @@ int wc_AesCcmEncrypt_silabs (Aes* aes, byte* out, const byte* in, word32 sz,
{
sl_status_t status;
if ((in == NULL) || (out == NULL) || (iv == NULL) || (authTag == NULL) ||
(ivSz < CCM_NONCE_MIN_SZ) || (ivSz > CCM_NONCE_MAX_SZ) ||
(authIn == NULL && authInSz != 0) || (aes == NULL)) {
return BAD_FUNC_ARG;
}

{
word32 lenSz = (word32)WC_AES_BLOCK_SIZE - 1U - ivSz;
/* With a large nonce, B[] runs out of room to represent inSz, and beyond
* that, the counter itself can wrap.
*/
if ((lenSz < sizeof(sz)) &&
(sz >= ((word32)1 << (lenSz * 8))))
{
return AES_CCM_OVERFLOW_E;
}
}

status = sl_se_ccm_encrypt_and_tag(
&(aes->ctx.cmd_ctx),
&(aes->ctx.key),
Expand All @@ -301,10 +314,23 @@ int wc_AesCcmDecrypt_silabs (Aes* aes, byte* out, const byte* in, word32 sz,
{
sl_status_t status;
if ((in == NULL) || (out == NULL) || (iv == NULL) || (authTag == NULL) ||
(ivSz < CCM_NONCE_MIN_SZ) || (ivSz > CCM_NONCE_MAX_SZ) ||
(authIn == NULL && authInSz != 0) || (aes == NULL)) {
return BAD_FUNC_ARG;
}

{
word32 lenSz = (word32)WC_AES_BLOCK_SIZE - 1U - ivSz;
/* With a large nonce, B[] runs out of room to represent inSz, and beyond
* that, the counter itself can wrap.
*/
if ((lenSz < sizeof(sz)) &&
(sz >= ((word32)1 << (lenSz * 8))))
{
return AES_CCM_OVERFLOW_E;
}
}

status = sl_se_ccm_auth_decrypt(
&(aes->ctx.cmd_ctx),
&(aes->ctx.key),
Expand Down
23 changes: 18 additions & 5 deletions wolfcrypt/src/port/ti/ti-aes.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ static int AesAuthSetKey(Aes* aes, const byte* key, word32 keySz)
static int AesAuthArgCheck(Aes* aes, byte* out, const byte* in, word32 inSz,
const byte* nonce, word32 nonceSz,
const byte* authTag, word32 authTagSz,
word32 *M, word32 *L)
word32 *M, word32 *L, int mode)
{
if (aes == NULL || nonce == NULL || authTag == NULL)
return BAD_FUNC_ARG;
Expand Down Expand Up @@ -376,6 +376,19 @@ static int AesAuthArgCheck(Aes* aes, byte* out, const byte* in, word32 inSz,
default:
return BAD_FUNC_ARG;
}

if (mode == AES_CFG_MODE_CCM) {
word32 lenSz = (word32)WC_AES_BLOCK_SIZE - 1U - nonceSz;
/* With a large nonce, B[] runs out of room to represent inSz, and beyond
* that, the counter itself can wrap.
*/
if ((lenSz < sizeof(inSz)) &&
(inSz >= ((word32)1 << (lenSz * 8))))
{
return AES_CCM_OVERFLOW_E;
}
}

return 0;
}

Expand Down Expand Up @@ -468,8 +481,8 @@ static int AesAuthEncrypt(Aes* aes, byte* out, const byte* in, word32 inSz,
word32 tmpTag[WC_AES_BLOCK_SIZE/sizeof(word32)];

ret = AesAuthArgCheck(aes, out, in, inSz, nonce, nonceSz, authTag,
authTagSz, &M, &L);
if (ret == WC_NO_ERR_TRACE(BAD_FUNC_ARG)) {
authTagSz, &M, &L, mode);
if (ret != 0) {
return ret;
}
if ((authIn == NULL) && (authInSz > 0)) {
Expand Down Expand Up @@ -571,8 +584,8 @@ static int AesAuthDecrypt(Aes* aes, byte* out, const byte* in, word32 inSz,
word32 tmpTag[WC_AES_BLOCK_SIZE/sizeof(word32)];

ret = AesAuthArgCheck(aes, out, in, inSz, nonce, nonceSz, authTag,
authTagSz, &M, &L);
if (ret == WC_NO_ERR_TRACE(BAD_FUNC_ARG)) {
authTagSz, &M, &L, mode);
if (ret != 0) {
return ret;
}
if ((authIn == NULL) && (authInSz > 0)) {
Expand Down
Loading
Loading