Skip to content

Fix integer underflow in AES-GCM key/data unwrap size calculations#296

Merged
dgarske merged 1 commit intowolfSSL:mainfrom
sameehj:fix
Mar 17, 2026
Merged

Fix integer underflow in AES-GCM key/data unwrap size calculations#296
dgarske merged 1 commit intowolfSSL:mainfrom
sameehj:fix

Conversation

@sameehj
Copy link
Contributor

@sameehj sameehj commented Mar 11, 2026

Multiple AES-GCM unwrap functions compute payload sizes by subtracting header sizes from wrappedKeySz/wrappedDataSz without first verifying the wrapped size is large enough. Since these are uint16_t operations, an undersized input wraps to a large value, causing out-of-bounds reads and writes via wc_AesGcmDecrypt and memcpy.

Add bounds checks before each subtraction in:

  • _AesGcmKeyUnwrap
  • _AesGcmDataUnwrap
  • _HandleKeyUnwrapAndExportRequest
  • _HandleKeyUnwrapAndCacheRequest
  • _HandleDataUnwrapRequest

Add regression tests that send undersized wrappedKeySz/wrappedDataSz through all unwrap client APIs and assert WH_ERROR_BADARGS.

Multiple AES-GCM unwrap functions compute payload sizes by subtracting
header sizes from wrappedKeySz/wrappedDataSz without first verifying the
wrapped size is large enough. Since these are uint16_t operations, an
undersized input wraps to a large value, causing out-of-bounds reads and
writes via wc_AesGcmDecrypt and memcpy.

Add bounds checks before each subtraction in:
  - _AesGcmKeyUnwrap
  - _AesGcmDataUnwrap
  - _HandleKeyUnwrapAndExportRequest
  - _HandleKeyUnwrapAndCacheRequest
  - _HandleDataUnwrapRequest

Add regression tests that send undersized wrappedKeySz/wrappedDataSz
through all unwrap client APIs and assert WH_ERROR_BADARGS.

Signed-off-by: Sameeh Jubran <sameeh@wolfssl.com>
@padelsbach
Copy link
Contributor

padelsbach commented Mar 16, 2026

There are many repetitive checks in this PR and #298. Consider unifying the logic into a common header.

There is a GCC builtin for safe add/subtract, and then fall back when not available. Eg:

#if defined(__has_builtin)
#  if __has_builtin(__builtin_add_overflow)
#    define HAVE_BUILTIN_OVERFLOW 1
#  endif
#elif defined(__GNUC__)
#  define HAVE_BUILTIN_OVERFLOW 1
#endif

static inline bool add_u64(uint64_t a, uint64_t b, uint64_t *r)
{
#if HAVE_BUILTIN_OVERFLOW
    return __builtin_add_overflow(a, b, r);
#else
    *r = a + b;
    return *r < a;
#endif
}

static inline bool sub_u64(uint64_t a, uint64_t b, uint64_t *r)
{
#if HAVE_BUILTIN_OVERFLOW
    return __builtin_sub_overflow(a, b, r);
#else
    *r = a - b;
    return a < b;
#endif
}

etc etc ...

And then the code becomes much more concise:

    if (add_u64(foo, bar, &sum)) {
        return BAD_ARG;
    }
    /* no overflow in sum, carry on */

@dgarske dgarske removed the request for review from AlexLanzano March 16, 2026 22:42
@dgarske dgarske merged commit 3e4549e into wolfSSL:main Mar 17, 2026
51 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants