From 948b4d09c12c8027c461fef202ddd3e827e1eb5b Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Mon, 3 Aug 2026 15:36:12 +0200 Subject: [PATCH 1/3] pkcs7: fix AuthEnvelopedData bufferPt leak in stream teardown wc_PKCS7_ResetStream()/wc_PKCS7_FreeStream() freed stream->aad, tag, nonce, buffer, and key but never stream->bufferPt, the encryptedContent buffer wc_PKCS7_DecodeAuthEnvelopedData() stashes across WANT_READ re-entries. When the decode is abandoned mid-stream (e.g. malformed input drives the parser into a pending WANT_READ state and the caller then calls wc_PKCS7_Free()) that allocation was never reclaimed. Found under LeakSanitizer by feeding wc_PKCS7_DecodeAuthEnvelopedData single-byte-corrupted and truncated copies of a valid AES128GCM AuthEnvelopedData message built with wc_PKCS7_EncodeAuthEnvelopedData; a corrupted length field reliably drove it into the leaking state. Fix: free/null stream->bufferPt in wc_PKCS7_ResetStream() alongside the other stream buffers. This also required nulling stream->bufferPt right after wc_PKCS7_DecodeAuthEnvelopedData's own successful-decrypt path frees the same pointer, since that path used to leave the stream pointer dangling and rely on the caller not resetting the stream again before it was overwritten - otherwise ResetStream would double-free it. Verified: the same corruption/truncation sweep under -fsanitize=address,undefined,leak now reports no leaks and no double-frees. ./wolfcrypt/test/testwolfcrypt and ./tests/unit.test --group pkcs7 --group pkcs7_sd --group pkcs7_ed --group pkcs7_sed --group pkcs7_cd pass on the ASan build. A full ./configure --enable-all && make && ./wolfcrypt/test/testwolfcrypt && ./tests/unit.test --api build also passes (0 failed, 1758 passed, 338 skipped). --- wolfcrypt/src/pkcs7.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index 305775d362a..e568397816b 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -245,15 +245,22 @@ static void wc_PKCS7_ResetStream(wc_PKCS7* pkcs7) XFREE(pkcs7->stream->tag, pkcs7->heap, DYNAMIC_TYPE_PKCS7); XFREE(pkcs7->stream->nonce, pkcs7->heap, DYNAMIC_TYPE_PKCS7); XFREE(pkcs7->stream->buffer, pkcs7->heap, DYNAMIC_TYPE_PKCS7); + /* stream->bufferPt holds the AuthEnvelopedData encryptedContent + * buffer across WANT_READ re-entries. wc_PKCS7_DecodeAuthEnveloped + * Data() only frees it on its own error paths; if the stream is torn + * down while a decode is still pending (e.g. wc_PKCS7_Free() called + * after a WANT_READ), it must be freed here or it leaks. */ + XFREE(pkcs7->stream->bufferPt, pkcs7->heap, DYNAMIC_TYPE_PKCS7); /* stream->key is always allocated with MAX_ENCRYPTED_KEY_SZ */ if (pkcs7->stream->key != NULL) ForceZero(pkcs7->stream->key, MAX_ENCRYPTED_KEY_SZ); XFREE(pkcs7->stream->key, pkcs7->heap, DYNAMIC_TYPE_PKCS7); - pkcs7->stream->aad = NULL; - pkcs7->stream->tag = NULL; - pkcs7->stream->nonce = NULL; - pkcs7->stream->buffer = NULL; - pkcs7->stream->key = NULL; + pkcs7->stream->aad = NULL; + pkcs7->stream->tag = NULL; + pkcs7->stream->nonce = NULL; + pkcs7->stream->buffer = NULL; + pkcs7->stream->bufferPt = NULL; + pkcs7->stream->key = NULL; /* reset values, note that content and tmpCert are saved */ pkcs7->stream->maxLen = 0; @@ -16424,6 +16431,9 @@ int wc_PKCS7_DecodeAuthEnvelopedData(wc_PKCS7* pkcs7, byte* in, ForceZero(encryptedContent, (word32)encryptedContentSz); XFREE(encryptedContent, pkcs7->heap, DYNAMIC_TYPE_PKCS7); encryptedContent = NULL; + #ifndef NO_PKCS7_STREAM + pkcs7->stream->bufferPt = NULL; + #endif ForceZero(decryptedKey, MAX_ENCRYPTED_KEY_SZ); XFREE(decryptedKey, pkcs7->heap, DYNAMIC_TYPE_PKCS7); decryptedKey = NULL; From 24a207826db06c970b3485919718a59073ab6ba1 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 4 Aug 2026 07:16:43 +0200 Subject: [PATCH 2/3] wc_port: return the source length from wc_strlcpy wc_strlcpy returned the number of bytes it copied, so a truncating call reported dstSize - 1 and a caller could not tell a truncated copy from an exact fit. strlcpy(3) instead returns the length of src -- the length the copy would have needed -- which is what makes the documented truncation check (ret >= dstSize) work. The doxygen comment in doc/dox_comments/header_files/types.h already specifies "Length of source string", so this brings the implementation in line with its own documentation rather than changing the contract. Walking the remainder of src also gives wc_strlcat the length it attempted in its truncating path, matching strlcat(3). No in-tree caller uses either return value. Verified against the OpenBSD implementation over a matrix of source strings and destination sizes (including dstSize 0): identical return values and identical destination buffers in all cases. --- wolfcrypt/src/wc_port.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/wolfcrypt/src/wc_port.c b/wolfcrypt/src/wc_port.c index 99be1cb3d96..34c09cb36db 100644 --- a/wolfcrypt/src/wc_port.c +++ b/wolfcrypt/src/wc_port.c @@ -1550,18 +1550,26 @@ char* wc_strsep(char **stringp, const char *delim) #ifdef USE_WOLF_STRLCPY size_t wc_strlcpy(char *dst, const char *src, size_t dstSize) { - size_t i; + size_t i = 0; - if (!dstSize) - return 0; + if (dstSize != 0) { + /* Always have to leave a space for NULL */ + for (; i < (dstSize - 1) && *src != '\0'; i++) { + *dst++ = *src++; + } + *dst = '\0'; + } - /* Always have to leave a space for NULL */ - for (i = 0; i < (dstSize - 1) && *src != '\0'; i++) { - *dst++ = *src++; + /* strlcpy() returns the length of src, not the number of bytes copied, so + * that a caller can detect truncation with (ret >= dstSize). Walk whatever + * did not fit -- src already points at the first byte not copied, and at + * the whole string when dstSize was 0 (which writes nothing). */ + while (*src != '\0') { + i++; + src++; } - *dst = '\0'; - return i; /* return length without NULL */ + return i; /* length of src, excluding the NULL */ } #endif /* USE_WOLF_STRLCPY */ From 40f191717cbfd015c238afa0fdacfe049f5f3a96 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 4 Aug 2026 07:23:09 +0200 Subject: [PATCH 3/3] wc_port: bound the dst scan in wc_strlcat and return the attempted length wc_strlcat measured dst with XSTRLEN(), which keeps reading until it finds a NUL regardless of dstSize. When dst holds no NUL within dstSize -- the case strlcat(3) explicitly bounds to prevent security problems in incorrect code -- that read runs off the end of the buffer. Under AddressSanitizer an unterminated 16-byte dst with dstSize 8 is a stack-buffer-overflow READ of size 17. Scan for the end of dst without passing dstSize. If no NUL is found the length of dst is taken to be dstSize, nothing is appended and dst is left un-terminated because there is no room for the NUL, matching strlcat(3). The return value is now the total length attempted in every case: the initial length of dst plus the length of src. The dstSize == 0 case returns the length of src rather than 0, since nothing can be appended and that is still the length the call tried to create. Verified against the OpenBSD implementation over a matrix of dst values, src values and destination sizes, including dst buffers containing no NUL at all: 196 cases, identical return values and identical destination buffers throughout. --- wolfcrypt/src/wc_port.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/wolfcrypt/src/wc_port.c b/wolfcrypt/src/wc_port.c index 34c09cb36db..ca45e15f9d2 100644 --- a/wolfcrypt/src/wc_port.c +++ b/wolfcrypt/src/wc_port.c @@ -1576,16 +1576,26 @@ size_t wc_strlcpy(char *dst, const char *src, size_t dstSize) #ifdef USE_WOLF_STRLCAT size_t wc_strlcat(char *dst, const char *src, size_t dstSize) { - size_t dstLen; + size_t dstLen = 0; - if (!dstSize) - return 0; - - dstLen = XSTRLEN(dst); + /* Find the end of dst without going past dstSize. XSTRLEN() would run off + * the end of a dst that holds no NUL within dstSize -- the very case this + * bound exists to contain. */ + while (dstLen < dstSize && dst[dstLen] != '\0') { + dstLen++; + } - if (dstSize < dstLen) - return dstLen + XSTRLEN(src); + if (dstLen == dstSize) { + /* No NUL within dstSize: the length of dst is taken to be dstSize, + * nothing is appended, and dst is left un-terminated because there is + * no room for the NUL. Only reachable when dstSize is wrong or dst is + * not a C string; returning here is what stops the append from running + * off the end. */ + return dstSize + XSTRLEN(src); + } + /* Total length attempted: the initial length of dst plus the length of + * src, which is what wc_strlcpy() returns. */ return dstLen + wc_strlcpy(dst + dstLen, src, dstSize - dstLen); } #endif /* USE_WOLF_STRLCAT */