[Bug]: Integer overflow in hash update could mess up the heap (hash.c & ti-hash.c)

[zulfff]

Contact Details

arjunaajalahla100@gmail.com (pelioro)

Version

main branch

Description

Found the same sketchy integer overflow pattern in two more files. Check these out:

File 1: wolfcrypt/src/hash.c around line 1957

if (*len < *used + inSz) {  // [OVERFLOW]
    *msg = XMALLOC(*used + inSz, heap, ...);  // [SMALL ALLOC]
}
XMEMCPY(*msg + *used, in, inSz);  // [HEAP OVERFLOW]

File 2: wolfcrypt/src/port/ti/ti-hash.c around line 81

if (hash->len < hash->used+len) {  // [OVERFLOW]
    p = XMALLOC(hash->used+len, NULL, ...);  // [SMALL ALLOC]
}
XMEMCPY(hash->msg+hash->used, data, len);  // [HEAP OVERFLOW]

Same deal as the SE050 issue (#9951) - if used is near 4GB and len causes addition to wrap around, we get tiny allocation but full write = heap overflow.

How to trigger

Build with hash keep feature:

./configure --enable-hashkeep
# or in user_settings.h:
#define WOLFSSL_HASH_KEEP

For TI hash, need TI hardware with:

#define WOLFSSL_TI_HASH

Same bug pattern as #9951, just in different files. Low severity but should be patched to keep the code consistent and robust. Better safe than sorry right?

Reproduction steps

Repro steps

Theoretical repro (not really exploitable irl due to memory limits):

1. Init hash context
2. Keep calling hash update until used nears 4GB
3. Pass inSz that causes used + inSz to overflow
4. Crash - small alloc, big write = heap corruption

Real talk on severity

• Impact: Low - Need ~4GB hash input which will OOM on most systems before hitting overflow
• Risk: Still worth fixing for defense-in-depth (same pattern got fixed in SE050 Fix potential overflows in used size calculation in generic, TI and SE050 hash functions. #9954)
• Exploitable: Nah, not really, but it's the same bad pattern that was already patched once

Suggested fix

Just add overflow check before the addition (same fix as SE050 #9954):

// For hash.c:1957
if (!WC_SAFE_SUM_WORD32(*used, inSz, tmpSz)) {
    return BUFFER_E;  // overflow would happen, bail out
}

// For ti-hash.c:81  
if (!WC_SAFE_SUM_WORD32(hash->used, len, tmpSz)) {
    return BAD_FUNC_ARG;  // overflow would happen, bail out
}

Relevant log output

[kareem-wolfssl]

Thanks for the report @AryzXploit. I've updated #9954 to address these as well. Let me know what you think.

[zulfff]

@kareem-wolfssl solid fix, thanks for handling all three files.

re the copilot comments - honestly the current code is fine. BAD_FUNC_ARG is what wolfSSL uses everywhere for invalid params, and splitting it into separate checks just adds lines for no real benefit.

using WC_SAFE_SUM_WORD32 is way cleaner than the manual overflow checks i was thinking of. good call.

(yeah ti-hash.c still has that weird NULL check for zero-length updates but that's pre-existing behavior so not your problem)

lgtm, ship it whenever