Memory safety code review: 17 findings across compiled sources

[cpsource]

Code review of all compiled sources in a --enable-harden-tls --disable-oldtls build, focused exclusively on buffer overflows, use-after-free, and memory leaks. At the time of this review, none of the findings below have existing issues or pull requests filed against them in the wolfSSL repository.

High Severity

1. Integer underflow in DecryptTls13 (buffer overflow)

src/tls13.c:2969

word16 dataSz = sz - ssl->specs.aead_mac_size underflows if sz < aead_mac_size, wrapping to a large value passed directly to wc_AesGcmDecrypt/wc_AesCcmDecrypt. The function has no internal guard and relies entirely on callers to enforce the minimum size. This is reachable from network input on every TLS 1.3 connection.

Medium Severity

2. Heap buffer overflow in dual-alg cert SAPKI copy

wolfcrypt/src/asn.c:9157

XMEMCPY(decodedPubKey, der->sapkiDer, der->sapkiLen) copies into a buffer allocated as MAX_PUBLIC_KEY_SZ, but sapkiLen is derived from untrusted certificate input and is not checked against that limit. Requires WOLFSSL_DUAL_ALG_CERTS.

3. Stack buffer overflow in ConfirmSignature DSA path

wolfcrypt/src/asn.c:16213-16247

The DSA case only checks sigSz >= DSA_MIN_SIG_SIZE (lower bound). Unlike the RSA case, there is no upper bounds check against MAX_ENCODED_SIG_SZ. Under WOLFSSL_NO_MALLOC, sigCpy is a stack buffer of that size, and an oversized DSA signature from a crafted certificate can overflow it.

4. Double-free in DupSSL on poly1305 allocation failure

src/ssl.c:905-922

DupSSL() copies ssl->encrypt to dup->encrypt via XMEMCPY (transferring cipher object pointers), then if poly1305 allocation fails, returns MEMORY_E before zeroing ssl->encrypt. Both structs hold the same cipher pointers, leading to a double-free when both are eventually freed. Triggerable under memory pressure with ChaCha20-Poly1305 and wolfSSL_write_dup().

5. Out-of-bounds read in DoTls13CertificateVerify dual-alg path

src/tls13.c:10697-10703

When parsing dual-algorithm certificate signatures, the second ato16 read is not bounds-checked against args->sz. If the first inner length field is crafted to be close to args->sz, the second read occurs past the validated signature data region.

6. Memory leak and stale state in fragmented handshake reassembly

src/internal.c:18865-18870

When EarlySanityCheckMsgReceived fails during fragmented handshake message reassembly, pendingMsg is not freed and pendingMsgSz/pendingMsgOffset are not reset. The buffer persists until connection teardown, and the stale state can cause follow-on issues if the connection continues processing after the error.

7. NULL dereference before NULL check in mp_sqrtmod_prime

wolfcrypt/src/ecc.c:15177-15178

mp_init_multi is called on XMALLOC'd pointers before the NULL checks at line 15183. If any of the 10 allocations returns NULL, mp_init_multi dereferences it, causing a deterministic crash. This is triggerable under memory pressure.

8. Memory leak pattern in SetKeys() across ~15 cipher types

src/keys.c:2400-3420

When allocating enc/dec cipher object pairs, if the enc object is allocated successfully but the dec allocation or initialization fails, the function returns MEMORY_E without freeing the enc object. This pattern repeats for ARC4, 3DES, ChaCha, AES-CBC, AES-GCM, AES-CCM, Camellia, HMAC, and others.

Low Severity

9. Out-of-bounds read in NextCert

src/tls13.c:8770-8781

No check that *idx + 3 <= length before c24to32, nor that the resulting length stays in bounds. Processes local certificate chain data, not directly attacker-controlled.

10. Undersized allocation in DoTls13CertificateRequest

src/tls13.c:5876

sizeof(CertReqCtx) + len - 1 when len == 0 allocates 1 byte less than the struct size. Not exploitable since 0 bytes are written in this case.

11. Integer underflow in SanityCheckCipherText ETM path

src/internal.c:21243

encryptSz - MacSize(ssl) is computed before the minLength check. The secondary bounds check catches it, but the ordering is incorrect.

12. Byte overflow in DoCertificateStatus OCSP_MULTI loop counter

src/internal.c:17513

byte idx wraps at 255. Bounded by MAX_CHAIN_DEPTH in practice, but the loop has no explicit iteration limit.

13. Integer overflow in AllocDer on 32-bit platforms

wolfcrypt/src/asn.c:22754

sizeof(DerBuffer) + length can wrap if length is near UINT32_MAX, causing a small allocation followed by a large XMEMSET.

14. Integer overflow in wc_DerToPemEx bounds check on 32-bit platforms

wolfcrypt/src/asn.c:23457

headerLen + footerLen + derSz sum can wrap, bypassing the outSz bounds check.

15. Integer overflow in wc_XChaCha20Poly1305_crypt_oneshot on 32-bit long int platforms

wolfcrypt/src/chacha20_poly1305.c:376-378

(long int)src_len + POLY1305_DIGEST_SIZE truncates on platforms where long int is 32-bit but size_t is 64-bit.

16. Memory leak in STM32 GCM decrypt path

wolfcrypt/src/aes.c:10344-10363

authInPadded is leaked if wolfSSL_CryptHwMutexLock() fails after allocation. Requires STM32 hardware crypto build.

17. Missing null terminator in wolfSSL_get_shared_ciphers()

src/ssl.c:1273-1284

When called with len=0, returns buf without writing a null terminator. Callers treating the return value as a C string will read uninitialized memory.

[rlm2002]

Hi @cpsource,

I have opened PRs #10079 and #10091 to address most of your issues.

Issues not addressed:
6. A failure from EarlySanityCheckMsgReceived() triggers a fatal alert. Connection will not be continued. The pendingMsg will be freed on close.
7. mp_init_multi() allows NULL arguments.
8. SetKeys() returning MEMORY_E will propagate up through the connection until it fails and the pointer will be freed with the ssl object.
13. length is verified before calling AllocDer() by size > MAX_HANDSHAKE_SZ. A wrap caused by calling the public API wc_AllocDer() requires application level misuse.
14. Similarly to issue 13, length is bounded by size > MAX_HANDSHAKE_SZ before calling wc_DerToPemEx(). Calling the function directly is an application level issue.

Please let me know if this fixes the issues on your end.

Best,
Ruby

[rlm2002]

@cpsource,

Please see that the previously mentioned PRs have been merged, so I will be closing this ticket. None of these issues will be treated as vulnerabilities, but if you think otherwise feel free to send a functional reproducer to support@wolfssl.com and reference this issue number.

By the way, we ask that GitHub issues is not used for security reports. In the future, please send them to support@wolfssl.com!

Ruby