sniffer: copy handshake hashes with the wc_*Copy APIs

[yosuke-wolfssl]

Problem

HashCopy() in src/sniffer.c copied handshake hash contexts with raw
XMEMCPY. Hash contexts own heap state in several configurations — the W
scratch buffer under WOLFSSL_SMALL_STACK_CACHE, the msg buffer under
WOLFSSL_HASH_KEEP, plus async, crypto-callback and hardware contexts. Two
defects followed:

• The destinations (session->sslServer->hsHashes and
  session->sslClient->hsHashes) come from wolfSSL_new() →
  InitHandshakeHashes() and already own allocated state. XMEMCPY leaked
  both and replaced the pointers with the source's, so one buffer was aliased
  by two independently freed WOLFSSL objects — a double free once both are
  released.
• The source HsHashes wrapper was XMEMSET and XFREEd without ever
  running wc_*Free, discarding the pointers it owned.

Reachable by any sniffer processing a TLS 1.2-or-below handshake that
negotiates extended master secret.

src/internal.c already documents this exact hazard for
InitHandshakeHashesAndCopy(). The sniffer's private HsHashes is a
different struct and needed the same treatment applied locally.

Fix (src/sniffer.c)

• HashCopy() now calls wc_ShaCopy(), wc_Md5Copy(),
  wc_Sha256Copy() and wc_Sha384Copy(), accumulating into a ret it
  returns. These APIs free the destination internally and re-allocate
  destination-owned state, so source and both destinations end up disjoint.
• HashFree(), a new counterpart to HashInit(), releases the wrapper's
  hash members and tolerates a NULL argument. It is called on every
  session->hash teardown path, each of which previously ran a bare XFREE:

Site	Context
FreeSnifferSession()	session teardown
ProcessServerHello()	EMS not negotiated
DoHandShake(), client_key_exchange	after the copy
CreateSession()	HashInit() failure

The XMEMSET scrub at the client_key_exchange site is retained and now runs
after HashFree(): wc_ShaFree() and wc_Md5Free() do not zero their
structs, so it still wipes the SHA-1 and MD5 transcript state.

Closes f-13332 and f-13333.

Verification

• Builds clean, no new warnings, with --enable-sniffer --enable-smallstackcache -DWOLFSSL_HASH_KEEP and with plain --enable-sniffer.
• scripts/sniffer-testsuite.test passes in both configs; make check 6
  passed, 5 skipped, 0 failed.
• Negative control: reverting only HashCopy() to XMEMCPY makes ASan report
  a heap use-after-free on the SHA-256 W buffer via Transform_Sha256,
  confirming both the aliasing and that the captures reach the EMS path.

Test coverage

The existing TLS 1.2 captures do reach the changed code on the success path:
HashCopy() runs twice per session (server and client) and the
client_key_exchange HashFree() runs, both confirmed under ASan. The TLS 1.3
captures do not reach HashCopy() at all.

Three branches remain uncovered and are not addressed here:

Path	Why the suite misses it
HashCopy() nonzero return	Needs a wc_*Copy() failure; the else in DoHandShake() was dead before this change and is now live
HashFree() in CreateSession()	Only runs when HashInit() fails
HashFree() in ProcessServerHello()	Only runs when EMS is not negotiated; every capture is generated by wolfSSL peers, which negotiate EMS by default

The first two need allocation-failure injection the sniffer has no harness for;
the third needs a TLS 1.2 capture generated with EMS disabled on both peers.

[Copilot]

🟢 Approval recommended

The changes match existing internal patterns (e.g., InitHandshakeHashesAndCopy() in src/internal.c) and correctly address confirmed memory-ownership hazards without introducing new unchecked failure paths.

Pull request overview

This pull request fixes unsafe copying and teardown of handshake hash contexts in the TLS sniffer when Extended Master Secret (EMS) is negotiated, replacing raw struct XMEMCPY with the wc_*Copy() APIs and adding proper hash-context cleanup to prevent leaks and double-frees.

Changes:

• Replace HashCopy()’s raw XMEMCPY of hash contexts with wc_ShaCopy(), wc_Md5Copy(), wc_Sha256Copy(), and wc_Sha384Copy() and propagate copy errors via a return code.
• Add HashFree() to properly release the sniffer’s private HsHashes members, and call it on all session teardown paths that previously only XFREE’d the wrapper.
• Preserve the existing post-copy XMEMSET scrub while ensuring heap-owned hash state is freed correctly first.

File summaries

File	Description
src/sniffer.c	Reworks EMS handshake-hash copy/free logic to use wc_*Copy() and wc_*Free() for correct ownership and teardown of hash contexts.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 0
• Review effort level: Lite

---

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

[github-actions]

MemBrowse Memory Report

No memory changes detected for:

• gcc-arm-cortex-m0plus
• gcc-arm-cortex-m3
• gcc-arm-cortex-m4
• gcc-arm-cortex-m4-baremetal
• gcc-arm-cortex-m4-crypto-only
• gcc-arm-cortex-m4-dtls13
• gcc-arm-cortex-m4-min-ecc
• gcc-arm-cortex-m4-openssl-compat
• gcc-arm-cortex-m4-pkcs7
• gcc-arm-cortex-m4-pq
• gcc-arm-cortex-m4-rsa-only
• gcc-arm-cortex-m4-sp-math
• gcc-arm-cortex-m4-tls12
• gcc-arm-cortex-m4-tls13
• gcc-arm-cortex-m7
• gcc-arm-cortex-m7-pq
• gcc-arm-cortex-m7-tls13
• linuxkm-pie
• linuxkm-standard
• stm32-sim-stm32h753

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #11389

Scan targets checked: wolfssl-bugs, wolfssl-src
Findings: 1

Required changes (1)

CreateSession error paths after session->hash is set still leak the hash contexts

File: src/sniffer.c:5619
Function: CreateSession
Category: Resource leaks

The PR adds HashFree() only to the HashInit() failure path. The three later bailouts in the same function (session->context == NULL at 5617, and the two wolfSSL_new() failures at 5624/5630) run after session->hash = newHash and XFREE the session without freeing the wrapper or its hash contexts, leaking the SMALL_STACK_CACHE W and HASH_KEEP msg allocations. With an empty ServerList, every client SYN leaks. Adjacent to known finding #13333, which this PR otherwise closes.

Related known finding #13333 (similar but distinct): Both omit cleanup of initialized hash contexts, but #13333 is in FreeSnifferSession and frees only the wrapper during teardown, while this finding is in CreateSession bailouts that free neither the wrapper nor its contexts. The faulting operations and error paths differ, and fixing #13333 alone does not cover these direct XFREE(session) returns.

Recommendation: Call HashFree(session->hash) and XFREE(session->hash, NULL, DYNAMIC_TYPE_HASHES) on all three bailouts, or route them through FreeSnifferSession().

Referenced code: src/sniffer.c:5619-5623 (5 lines)

---

This review was generated automatically by Fenrir. Reported findings require changes before merge.

Fenrir's latest completed scan found no issues; clearing the prior automated change request.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #11389

Scan targets checked: wolfssl-bugs, wolfssl-src

Fenrir result: Approved ✅

No new issues found in the changed files.

Advisory only — this automated result does not count as a GitHub approval.