Fix cert chain size issue

[embhorn]

Description

In wolfssl_add_to_chain, the calculation len + CERT_HEADER_SZ + certSz uses word32, which could cause an overflow

Fixes zd21241

Testing

Added test case test_wolfSSL_add_to_chain_overflow

Checklist

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

[Copilot]

Pull request overview

Fixes a potential 32-bit integer overflow when extending a certificate chain in wolfssl_add_to_chain, and adds a regression test to ensure the overflow is rejected.

Changes:

• Add an overflow guard for len + CERT_HEADER_SZ + certSz in wolfssl_add_to_chain
• Add an API test that simulates a near-UINT32_MAX chain length to verify failure behavior

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
tests/api.c	Adds a regression test that forces the size calculation into an overflow scenario.
src/ssl_load.c	Adds bounds checks to prevent word32 overflow when computing the new chain allocation size.

Comments suppressed due to low confidence (1)

tests/api.c:1

• This test replaces ctx->certChain with a DerBuffer that doesn't appear to follow the same allocation/free pattern as production code (e.g., AllocDer commonly allocates DerBuffer and buffer in a specific way). Also, freeing the existing chain with XFREE(ctx->certChain, ...) may bypass any required freeing of ctx->certChain->buffer (depending on how DerBuffer instances are normally allocated). To make this test robust and avoid allocator/cleanup fragility, construct the fake chain using the same helper(s) used by production (e.g., an AllocDer-style path) and dispose of the original chain using the corresponding DerBuffer free routine (or explicitly free both buffer and the struct in the same way production does).

---

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

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

tests/api.c:1

• ctx->heap is dereferenced unconditionally when allocating fakeChain. If wolfSSL_CTX_new(...) fails, ExpectNotNull likely records the failure but execution continues, leading to a null dereference here. Wrap the remainder of the test body in an if (EXPECT_SUCCESS()) { ... } block (or return early) before using ctx / ctx->heap.
  tests/api.c:1
• ctx->certChain is a DerBuffer* and is typically created/freed via the DER buffer helpers (e.g., AllocDer + the matching free routine). Freeing it with XFREE risks leaking the embedded buffer or freeing with the wrong routine (depending on how the chain was allocated). Prefer using the same deallocation function used elsewhere for DerBuffer instances (the one that also releases certChain->buffer).
  tests/api.c:1
• fakeChain->buffer is pointed inside the same allocation as the struct. When the context is freed, the cleanup path for a DerBuffer commonly frees buffer separately from the struct; if that happens, this setup can trigger an invalid free / heap corruption. To keep the test safe, construct fakeChain in a way that matches the normal DerBuffer ownership model (e.g., allocate buffer independently, or use the standard DER allocation helper and then adjust the length for the overflow scenario).

---

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

[embhorn]

Jenkins retest this please

[embhorn]

Jenkins retest this please