Fix UAF in DH buffers

[padelsbach]

Description

Fixes the case where SetSSL_CTX aliases the serverDH_P and serverDH_G buffers with weOwnDH still at 0. A subsequent call to wolfSSL_CTX_SetTmpDH* would result in a dangling pointer.

Related to #11109, but with DH buffers. Note that refcounting is not performed here since the API uses plain byte buffers instead of structs/objects.

weOwnDH == 0 currently means either "CTX heap buffer" or "static named-group params" today. After this change, it only means the static case which removes ambiguity in the free paths.

Testing

Added unit tests.

Checklist

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

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #11115

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

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

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

[github-actions]

MemBrowse Memory Report

gcc-arm-cortex-m0plus

• FLASH: .text +2,020 B (+3.1%, 66,855 B / 262,144 B, total: 26% used)

gcc-arm-cortex-m3

• FLASH: .text +2,128 B (+1.7%, 125,651 B / 262,144 B, total: 48% used)

gcc-arm-cortex-m4

• FLASH: .text +2,624 B (+1.3%, 204,463 B / 262,144 B, total: 78% used)

gcc-arm-cortex-m4-baremetal

• FLASH: .text +1,920 B (+2.8%, 69,347 B / 262,144 B, total: 26% used)

gcc-arm-cortex-m4-crypto-only

• FLASH: .text +2,048 B (+1.2%, 177,962 B / 262,144 B, total: 68% used)

gcc-arm-cortex-m4-dtls13

• FLASH: .text +2,688 B (+1.5%, 185,596 B / 1,048,576 B, total: 18% used)

gcc-arm-cortex-m4-min-ecc

• FLASH: .text +1,920 B (+3.1%, 64,261 B / 262,144 B, total: 25% used)

gcc-arm-cortex-m4-openssl-compat

• FLASH: .rodata +216 B, .text +3,840 B (+0.5%, 779,540 B / 1,048,576 B, total: 74% used)

gcc-arm-cortex-m4-pkcs7

• FLASH: .text +2,304 B (+1.1%, 217,070 B / 262,144 B, total: 83% used)

gcc-arm-cortex-m4-pq

• FLASH: .text +2,688 B (+0.9%, 300,252 B / 1,048,576 B, total: 29% used)

gcc-arm-cortex-m4-rsa-only

• FLASH: .text +2,688 B (+0.8%, 330,800 B / 1,048,576 B, total: 32% used)

gcc-arm-cortex-m4-sp-math

• FLASH: .text +1,920 B (+3.1%, 64,261 B / 262,144 B, total: 25% used)

gcc-arm-cortex-m4-tls12

• FLASH: .text +2,176 B (+1.8%, 126,451 B / 262,144 B, total: 48% used)

gcc-arm-cortex-m4-tls13

• FLASH: .text +2,688 B (+1.1%, 240,993 B / 262,144 B, total: 92% used)

gcc-arm-cortex-m7

• FLASH: .text +2,624 B (+1.3%, 204,463 B / 262,144 B, total: 78% used)

gcc-arm-cortex-m7-pq

• FLASH: .text +2,688 B (+0.9%, 301,148 B / 1,048,576 B, total: 29% used)

gcc-arm-cortex-m7-tls13

• FLASH: .text +2,688 B (+1.1%, 241,057 B / 262,144 B, total: 92% used)

linuxkm-pie

• Data: __patchable_function_entries +272 B (+1.0%, 26,832 B)

linuxkm-standard

• Data: __patchable_function_entries +264 B (+0.5%, 49,640 B)

stm32-sim-stm32h753

• FLASH: .text +2,048 B (+1.1%, 188,208 B / 2,097,152 B, total: 9% used)

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #11115

Scan targets checked: wolfcrypt-rs-bugs, wolfssl-bugs, wolfssl-src
Findings: 6
5 finding(s) posted as inline comments (see file-level comments below)

Info (1)

Buffers struct comments still claim the CTX owns serverDH_P/serverDH_G

File: wolfssl/internal.h:5261
Function: Buffers
Category: Logic errors

CopySSL_CTX_DhParams makes the SSL object always own p/g when they come from the CTX, and the only remaining weOwnDH == 0 case (tls.c:5467) points at static DhParams tables that neither object owns. The "WOLFSSL_CTX owns, unless we own" comments now describe an ownership model that no longer exists, two lines above the new declaration.

Recommendation: Reword both comments to state that the SSL owns p/g when weOwnDH is set and otherwise points at static named-group parameters.

Referenced code: wolfssl/internal.h:5261-5262 (2 lines)

---

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

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #11115

Scan targets checked: wolfcrypt-rs-bugs, 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.

[padelsbach]

jenkins retest this please

[Copilot]

Pull request overview

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Fixes a use-after-free risk where WOLFSSL sessions could alias WOLFSSL_CTX DH parameter buffers (P/G) without ownership, then later observe dangling pointers when the context rotates/replaces DH parameters.

Changes:

• Introduce CopySSL_CTX_DhParams() to deep-copy DH (P/G) buffers from WOLFSSL_CTX into WOLFSSL sessions.
• Update SetSSL_CTX() and wolfSSL_set_accept_state() to use the copy helper instead of aliasing context buffers.
• Add/adjust unit tests to validate DH parameter copying, rotation safety, reuse behavior, and OOM handling.

Reviewed changes

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

Show a summary per file

File	Description
wolfssl/internal.h	Documents DH buffer ownership semantics and declares the new DH copy helper.
src/internal.c	Implements DH parameter deep-copying and adjusts resource cleanup behavior.
src/ssl_api_hs.c	Uses the new DH copy helper during wolfSSL_set_accept_state().
tests/api/test_tls13.h	Registers new TLS 1.3 DH-related tests.
tests/api/test_tls13.c	Adds TLS 1.3 tests for DH rotation and accept-state DH copying.
tests/api/test_ssl_hs.h	Registers new handshake tests for DH reuse and OOM behavior.
tests/api/test_ssl_hs.c	Updates existing DH expectations and adds new DH reuse/OOM coverage.

Suppressed comments (4)

src/internal.c:1

• Returning success when the context has no DH buffers leaves any existing ssl->buffers.serverDH_{P,G} state untouched. Since SetSSL_CTX() now relies on this helper, switching an SSL from a context-with-DH to a context-without-DH can retain stale DH parameters that no longer match the new context. Consider treating missing ctx DH params as a 'clear' operation: if ssl->buffers.weOwnDH is set, free existing P/G buffers, set both pointers to NULL, set both lengths to 0, and reset weOwnDH (and ensure the caller keeps options.haveDH consistent).
  src/internal.c:1
• When freeing client-side DH parameters in FreeHandshakeResources(), the code clears the pointers but leaves ssl->buffers.weOwnDH and the serverDH_{P,G}.length fields unchanged. This can leave the object in an inconsistent state (e.g., 'owns DH' but buffers are NULL), which is error-prone for later cleanup paths and any logic that consults lengths/ownership. After freeing, also reset weOwnDH to 0 and set both DH lengths to 0 (and keep options.haveDH consistent as applicable).
  src/internal.c:1
• CopySSL_CTX_DhParams is declared as WOLFSSL_LOCAL in wolfssl/internal.h, but the definition in src/internal.c omits WOLFSSL_LOCAL. To ensure consistent symbol visibility/linkage (and avoid unintentionally exporting the symbol on some toolchains), annotate the definition with WOLFSSL_LOCAL as well.
  src/internal.c:1
• Given the behavior change (sessions now copy DH params in SetSSL_CTX()), it would be useful to add a unit test that switches an existing WOLFSSL* between two contexts where the second context has no DH parameters. The test should assert that the SSL object's DH buffers/lengths/ownership are cleared (and haveDH is consistent), so it doesn't retain stale parameters from the previous context.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #11115

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

No new issues found in the changed files. ✅