Use refcount on DER buffer to prevent UAF

[padelsbach]

Description

This change fixes a use-after-free when the certificate is updated in the SNI callback (triggered by the clienthello). The certificate and certChain buffer are pointed to by both the WOLFSSL object and the WOLFSSL_CTX object, and a call to wolfSSL_CTX_use_certificate_file() triggers a free+null of the pointers in CTX but not the SSL object.

Observable failure is an ASAN read failure in wolfSSL_accept_TLSv13. Requires HAVE_SNI enabled.

Fix is to add reference counter on the shared DerBuffer.

Fixes zd 22107.

Testing

TDD using new unit test.

Checklist

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

[github-actions]

MemBrowse Memory Report

gcc-arm-cortex-m0plus

• FLASH: .text +32 B (+0.0%, 64,187 B / 262,144 B, total: 24% used)

gcc-arm-cortex-m3

• FLASH: .text +72 B (+0.1%, 122,521 B / 262,144 B, total: 47% used)

gcc-arm-cortex-m4

• FLASH: .text +64 B (+0.0%, 200,348 B / 262,144 B, total: 76% used)

gcc-arm-cortex-m4-baremetal

• FLASH: .text +64 B (+0.1%, 66,827 B / 262,144 B, total: 25% used)

gcc-arm-cortex-m4-dtls13

• FLASH: .text +64 B (+0.0%, 180,888 B / 1,048,576 B, total: 17% used)

gcc-arm-cortex-m4-pq

• FLASH: .text +64 B (+0.0%, 279,300 B / 1,048,576 B, total: 27% used)

gcc-arm-cortex-m4-rsa-only

• FLASH: .text +64 B (+0.0%, 324,960 B / 1,048,576 B, total: 31% used)

gcc-arm-cortex-m4-tls12

• FLASH: .text +64 B (+0.1%, 123,277 B / 262,144 B, total: 47% used)

gcc-arm-cortex-m4-tls13

• FLASH: .text +64 B (+0.0%, 235,982 B / 262,144 B, total: 90% used)

gcc-arm-cortex-m7

• FLASH: .text +64 B (+0.0%, 200,284 B / 262,144 B, total: 76% used)

gcc-arm-cortex-m7-pq

• FLASH: .text +64 B (+0.0%, 279,876 B / 1,048,576 B, total: 27% used)

gcc-arm-cortex-m7-tls13

• FLASH: .text +64 B (+0.0%, 236,046 B / 262,144 B, total: 90% used)

linuxkm-pie

• Data: __patchable_function_entries +8 B (+0.0%, 25,520 B)

linuxkm-standard

• Data: __patchable_function_entries +8 B (+0.0%, 48,168 B)
  No memory changes detected for:
• gcc-arm-cortex-m4-crypto-only
• gcc-arm-cortex-m4-min-ecc
• gcc-arm-cortex-m4-openssl-compat
• gcc-arm-cortex-m4-pkcs7
• gcc-arm-cortex-m4-sp-math
• stm32-sim-stm32h753

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #10860

Scan targets checked: wolfcrypt-bugs, wolfcrypt-rs-bugs, wolfcrypt-src, wolfssl-bugs, wolfssl-src

Findings: 2
2 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

[wolfSSL-Fenrir-bot]

🔵 [Low] RefDer silently discards wolfSSL_RefInc error, leaving aliased pointer dangling on mutex failure · Use-after-free / double-free

RefDer calls wolfSSL_RefInc and discards the error with (void)ref_err. In mutex-based builds (WOLFSSL_REFCNT_ERROR_RETURN), if wc_LockMutex fails the refcount is not incremented, so a subsequent FreeDer on the CTX-side buffer drops the count to zero and frees it while the SSL object still holds the aliased pointer — a UAF. Contrast with wolfSSL_RSA_up_ref (src/pk_rsa.c:362) which returns !err to signal failure.

Fix: Have RefDer return an int error code and propagate the ref_err value to callers so they can abort rather than proceed with an un-incremented alias.

[padelsbach]

fixed

[wolfSSL-Fenrir-bot]

🔵 [Low] SNI callback discards cert-reload return value, leaving UAF scenario unexercised on failure · Weak or missing assertions

wolfSSL_CTX_use_certificate_file is cast to void, so if the reload silently fails the cert pointer is never replaced, the aliased-buffer UAF condition is never triggered, and the handshake still completes — the test passes while providing no coverage of the fix.

Fix: Check the return value and propagate a non-zero *ad / non-zero return to force the test to fail if the cert reload does not succeed.

[padelsbach]

fixed

[padelsbach]

jenkins retest this please

[dgarske]

Skoll Multi-Scan Review

Modes: review + review-securityOverall recommendation: REQUEST_CHANGES
Findings: 4 total — 4 posted, 0 skipped
3 finding(s) posted as inline comments (see file-level comments below)
1 finding(s) not tied to a diff line (full detail below)

Posted findings

• [Medium] [review+review-security] No test coverage for add0/add1_chain_cert on an SSL that aliases the CTX chain — tests/api/test_tls13.c:3630-3693
• [Medium] [review+review-security] Public DerBuffer struct grows a new member (ABI/size change; FreeDer now requires AllocDer-initialized ref) — wolfssl/wolfcrypt/asn_public.h:241-246
• [Low] [review] RefDer treats NULL as success; FreeDer discards mutex-lock failure (leak-on-lock-failure) — wolfcrypt/src/asn.c:24720-24727

Findings not tied to a diff line

wolfssl_add_to_chain leaks aliased cert-chain reference (missed refcount release when weOwn==0)

File: src/ssl_load.c:4924-4929
Function: wolfssl_add_to_chain
Severity: High
Category: Resource Leak

This PR changes the ownership model so aliased cert/certChain buffers now hold a real reference. SetSSL_CTX_CertsAndKeys (src/internal.c:7180-7184) calls RefDer(ssl->buffers.certChain) when the SSL aliases the CTX chain, so an SSL holds a reference on ctx->certChain even though ssl->buffers.weOwnCertChain == 0 (refcount == 2). Every other pointer-drop site in the PR was converted to an UNCONDITIONAL FreeDer to release that reference (wolfSSL_certs_clear, ProcessUserChainRetain, ProcessBufferCertHandleDer, wolfSSL_ResourceFree). But wolfssl_add_to_chain was NOT converted: it still only frees the old chain when if (weOwn) { FreeDer(chain); } and then overwrites *chain. wolfSSL_add0_chain_cert / wolfSSL_add1_chain_cert (src/ssl_load.c:5225-5227) call it as wolfssl_add_to_chain(&ssl->buffers.certChain, ssl->buffers.weOwnCertChain, ...). When the chain is aliased (weOwn == 0), the aliased reference taken by RefDer is dropped WITHOUT a matching FreeDer: on wolfSSL_CTX_free the count decrements 2->1 and the CTX cert-chain buffer is never freed. Reachable via the public OpenSSL-compat APIs SSL_add0_chain_cert / SSL_add1_chain_cert (nginx/httpd) under OPENSSL_EXTRA + KEEP_OUR_CERT when the CTX loaded a certificate chain before the SSL was created. Leaks only on the success path of the add. The other two callers (lines 4970, 5162) pass weOwn=1 on a refcount-1 ctx->certChain, so unconditional free is also correct for them. Severity: review mode rated this High/BLOCK; review-security rated it Medium; stricter view kept.

Recommendation: Make the FreeDer(chain) unconditional (drop the if (weOwn) guard), matching the conversion applied to the other alias-drop sites in this PR. FreeDer now decrements a shared alias and only frees at refcount 0, so this is safe for both owned (weOwn=1 CTX callers) and aliased chains. The weOwn parameter then becomes redundant for the free decision and can be removed. Add a regression test that loads a chain on the CTX, creates an SSL (aliasing the chain), calls SSL_add1_chain_cert on the SSL, then frees both under ASAN/valgrind.

Referenced code: src/ssl_load.c:4924-4929 (6 lines)

---

Review generated by Skoll

[dgarske]

🟠 [Medium] No test coverage for add0/add1_chain_cert on an SSL that aliases the CTX chain · Missing Tests

The added test exercises only the leaf-certificate reload path (wolfSSL_CTX_use_certificate_file in the SNI callback), which correctly decrements the aliased reference via ProcessBufferCertHandleDer's now-unconditional FreeDer. The refcount change also affects the cert-CHAIN aliasing path, but there is no test that appends to an SSL's aliased chain via wolfSSL_add1_chain_cert / wolfSSL_add0_chain_cert: create an SSL from a CTX that loaded an intermediate chain (so ssl->buffers.certChain aliases ctx->certChain with refcount 2), then call wolfSSL_add1_chain_cert(ssl, x509) and free both under leak detection. That is exactly the path where the wolfssl_add_to_chain leak (see High finding) survives, which is why the leak was not caught. Severity: review Medium/SUGGEST, review-security Low; stricter view kept.

Fix: Add a test under OPENSSL_EXTRA + KEEP_OUR_CERT that (1) loads a certificate chain on the CTX, (2) creates a WOLFSSL so ssl->buffers.certChain aliases ctx->certChain, (3) calls wolfSSL_add1_chain_cert(ssl, x509) (or wolfSSL_add0_chain_cert), and (4) frees ssl then ctx, verifying no leak under ASAN/valgrind. This validates the chain-alias refcount balance and guards the fix for the High finding.

[dgarske]

🟠 [Medium] Public DerBuffer struct grows a new member (ABI/size change; FreeDer now requires AllocDer-initialized ref) · API / ABI

The public DerBuffer struct gains a wolfSSL_Ref ref member, correctly appended at the end (per wolfSSL struct convention, avoiding mid-struct ABI breakage). FreeDer now calls wolfSSL_RefDec(&der->ref, ...) on every buffer. Two consequences: (1) sizeof(DerBuffer) changes, so any application or FIPS-boundary build that allocates/embeds DerBuffer directly, or links against a prebuilt shared library, must be recompiled - a classic shared-library ABI break. (2) In mutex-backed refcount builds (!SINGLE_THREADED && !WOLFSSL_ATOMIC_OPS), a DerBuffer NOT created via AllocDer (which now runs wolfSSL_RefInit) has an uninitialized mutex and hits UB when freed with wc_FreeDer; every cert/key DerBuffer also now embeds and init/frees a mutex per AllocDer/FreeDer. All in-tree allocations go through AllocDer (verified: no sizeof(DerBuffer) allocation or raw XFREE of a DerBuffer struct exists outside wolfcrypt/src/asn.c), and the PR fixed the one internal offender (test_wolfSSL_add_to_chain_overflow now uses AllocDer/FreeDer); the stack DerBuffer rawDer in wolfcrypt/src/evp_pk.c is safe because it is never passed to FreeDer. Severity: review Medium/SUGGEST, review-security Info; stricter view kept.

Fix: Document in the ChangeLog/release notes the sizeof(DerBuffer) ABI change so downstream users rebuild rather than mix object files across the boundary, and add a contract note in the DerBuffer/FreeDer doxygen that a DerBuffer must be obtained from AllocDer/wc_AllocDer before being freed with FreeDer/wc_FreeDer. No functional code change required.

[dgarske]

🔵 [Low] RefDer treats NULL as success; FreeDer discards mutex-lock failure (leak-on-lock-failure) · Style

RefDer(NULL) returns 1 (success), which is intentional so aliasing a NULL ctx->certificate is a no-op; callers rely on this. Worth a brief note only: in the mutex build, a lock failure inside FreeDer's wolfSSL_RefDec sets isZero=0 and the buffer is intentionally not freed (leak-on-lock-failure), and ref_err is discarded via (void)ref_err;. This matches the library's existing refcount conventions (session/ctx refcounts), so no change is required, but the discarded error is easy to misread.

Fix: No functional change needed; consider a one-line comment at the FreeDer (void)ref_err; noting the deliberate leak-on-lock-failure behavior for parity with other refcounted frees.

[dgarske]

Do we really need to always add "wolfSSL_Ref ref;" on all builds? I am concerned about portability and code size growth.

[padelsbach]

Will redesign a solution to prevent/block this scenario rather than try to fix it. Closing this PR.