Various fixes

[kareem-wolfssl]

Description

Fix multiple missing bounds and length checks.

Fixes zd#21536, zd#21537, zd#21538, zd#21540, zd#21541

Testing

Built in tests, provided reproducers

Checklist

• added tests
• updated/added doxygen
• updated appropriate READMEs
• Updated manual and documentation

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Fixes multiple missing bounds/length checks across TLS 1.3 handshake, OCSP processing, and sniffer ClientHello parsing to prevent out-of-bounds reads/writes and invalid-length parsing.

Changes:

• Added maximum certificate extension count guards in TLS 1.3 certificate/OCSP flows.
• Added minimum-length checks before reading OPAQUE16-sized fields from buffers.
• Improved error return behavior/documentation for certificate status request buffer writing.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
src/tls13.c	Adds MAX_CERT_EXTENSIONS bounds checks around CSR-to-buffer writing and TLS 1.3 certificate sending.
src/tls.c	Adds OPAQUE16 minimum-length validation and caps OCSP request processing to MAX_CERT_EXTENSIONS.
src/sniffer.c	Adds OPAQUE16 minimum-length validation for several ClientHello extensions.
src/internal.c	Adds MAX_CERT_EXTENSIONS bounds checks while generating/sending OCSP status responses.

Comments suppressed due to low confidence (1)

src/internal.c:1

• This loop can still read past the end of chainOcspRequest because i is incremented inside &responses[++i]. After the last valid increment, the next while condition evaluates ssl->ctx->chainOcspRequest[i] with i == MAX_CERT_EXTENSIONS, causing an out-of-bounds read before the in-loop bounds check runs. Refactor to avoid pre-increment in the call (increment i separately after a bounds check) and ensure the bounds check occurs before any chainOcspRequest[i] dereference.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

This introduces a declaration (word16 ksLen) after a statement within the block. If this code is compiled in a C90/C89 mode (common for some embedded/toolchain configurations), it will not compile. Move word16 ksLen up with the other declarations at the start of the block and then assign to it after the length check.

Suggested change

	if (extLen < OPAQUE16_LEN)
	return BUFFER_ERROR;
	word16 ksLen = (word16)((input[0] << 8) | input[1]);
	word16 ksLen;
	if (extLen < OPAQUE16_LEN)
	return BUFFER_ERROR;
	ksLen = (word16)((input[0] << 8) | input[1]);

[Copilot]

For malformed key_share extensions, this new early return BUFFER_ERROR; bypasses the existing error-reporting path (SetError(...)) used for other length failures in the same block, which can lead to inconsistent diagnostics/state handling. Consider setting the sniffer error via SetError(...) before returning for extLen < OPAQUE16_LEN, consistent with the ksLen + OPAQUE16_LEN > extLen branch.

[Copilot]

The bound check narrows (1 + ssl->buffers.certChainCnt) to word16 before comparing to MAX_CERT_EXTENSIONS. If certChainCnt can exceed 65534, this cast can wrap and the check may incorrectly pass. Compare using a wider type (e.g., compute in word32/size_t without narrowing) and only cast after validation.

[Copilot]

This log message could be more actionable by including the limit (e.g., MAX_CERT_EXTENSIONS) and/or the observed index/count (i). That makes field debugging easier when the error is encountered.

Suggested change

	WOLFSSL_MSG("OCSP request cert chain exceeds maximum length.");
	WOLFSSL_ERROR_MSG_EX(
	"OCSP request cert chain exceeds maximum length: "
	"index=%d, limit=%d", i, MAX_CERT_EXTENSIONS);