Skip to content

Avoid writing into caller ikm buffer in wc_Tls13_HKDF_Extract#10938

Open
yosuke-wolfssl wants to merge 1 commit into
wolfSSL:masterfrom
yosuke-wolfssl:fix/f_6767
Open

Avoid writing into caller ikm buffer in wc_Tls13_HKDF_Extract#10938
yosuke-wolfssl wants to merge 1 commit into
wolfSSL:masterfrom
yosuke-wolfssl:fix/f_6767

Conversation

@yosuke-wolfssl

Copy link
Copy Markdown
Contributor

Description

wc_Tls13_HKDF_Extract() and wc_Tls13_HKDF_Extract_ex() write digest length zero bytes
into the caller supplied ikm buffer when ikmLen == 0, before any validation:

/* When length is 0 then use zeroed data of digest length. */
if (ikmLen == 0) {
    ikmLen = len;
    XMEMSET(ikm, 0, len);   /* writes 32, 48 or 64 bytes into the caller's buffer */
}

The size this requires is not implied by the length the caller passes, which is 0, so it
is an undocumented precondition. Saying "no input keying material" the natural way,
wc_Tls13_HKDF_Extract(prk, salt, saltLen, NULL, 0, WC_SHA256), runs XMEMSET(NULL, 0, 32)
and crashes. A non NULL buffer sized for the 0 length argument gets len bytes written
past the end. Both were reproduced under ASan as a SEGV on a NULL write and a
stack-buffer-overflow of WRITE of size 32.

The layer underneath already accepts the call that crashes here: wc_HKDF_Extract_ex()
guards with out == NULL || (inKey == NULL && inKeySz > 0), and the existing
test_wc_HKDF_NullKeyEdgeCases asserts a NULL key of length 0 is valid zero length IKM.
The TLS 1.3 wrapper was rejecting, by crashing, input its own callee supports.

No in tree caller can overflow. The call sites in src/tls13.c pass buffers of 64, 48 or
ENCRYPT_LEN bytes against a 32 or 48 byte digest, so this is an API boundary hazard for
external callers.

Addressed by f_6767.

Changes

wolfcrypt/src/kdf.c: when ikmLen == 0, point a const byte* localIkm alias at a local
zeroed tmp buffer instead of writing through the caller's pointer, mirroring the
tmp / localSalt idiom wc_HKDF_Extract_ex() already uses for a NULL salt. Validate
prk and ikm at entry. salt is deliberately left alone, since wc_HKDF_Extract_ex()
treats a NULL salt as "no salt" and substitutes a zeroed salt of digest length per RFC 5869
section 2.2; rejecting it here would make the wrapper stricter than its callee.

Behavior change: the function no longer zeroes the caller's ikm buffer when
ikmLen == 0. The derived PRK is bit identical, since both old and new code feed HKDF len
zero bytes and only the source buffer changed. No in tree caller depends on the side effect.

tests/api/test_hmac.c: adds test_wc_Tls13_HKDF_Extract_ZeroLenIkm, which fails before the
fix. A NULL ikm with ikmLen == 0 must succeed and derive the same PRK as an explicit
zero buffer; a digest length sentinel passed with ikmLen == 0 must come back untouched.
Covers both the wrapper and the _ex entry point src/tls13.c calls, every digest the
switch accepts, the BAD_FUNC_ARG cases, and under WOLF_CRYPTO_CB that a callback
receives the substituted zeroed IKM rather than the caller's buffer.

Doxygen: ikm was documented as "pointer to putput for keying material", describing it as an
output. Corrected, and documented that ikmLen == 0 substitutes a digest length zeroed IKM
per RFC 8446 section 7.1, which is not RFC 5869 extract with an empty IKM; those produce
different PRKs. Also fixed the _Example_ blocks, which passed 7 arguments to a 6 parameter
function with 0 landing on ikm. Japanese translations kept in sync.

Testing

testwolfcrypt and full unit.test pass with --enable-tls13 --enable-hkdf, with
--enable-cryptocb, under ASan and UBSan, and NO_HASH_WRAPPER compiles. The TLS 1.3 cipher
suite tests exercise the real DeriveEarlySecret and DeriveMasterSecret zero length
callers and are unaffected. Also compile checked on the FIPS and selftest #else path and
the WOLFSSL_DEBUG_TLS block.

Note for reviewers: FIPS module boundary

wolfcrypt/src/kdf.c is inside the FIPS module boundary. It sets FIPS_NO_WRAPPERS and
places code in the .fipsA$h / .fipsB$h segments under FIPS_VERSION3_GE(5,0,0), and
carries a wolfCrypt_FIPS_KDF_sanity hook. The frozen 5.2.1 and 5.2.4 bundles ship their own
copy of kdf.c, in segment .fipsA$m, which still contains the unfixed write. This fix
therefore does not reach FIPS users until it folds into a module revision, which is why the
new test skips FIPS and selftest builds.

@yosuke-wolfssl yosuke-wolfssl self-assigned this Jul 17, 2026
Copilot AI review requested due to automatic review settings July 17, 2026 02:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the TLS 1.3 HKDF Extract wrapper APIs to avoid writing into (or requiring) caller-provided IKM buffers when ikmLen == 0, aligning the wrapper behavior with the underlying HKDF implementation and preventing NULL dereferences / buffer overruns for external callers.

Changes:

  • Update wc_Tls13_HKDF_Extract() / _ex() to substitute a local zeroed IKM buffer when ikmLen == 0 (instead of writing through the caller’s ikm pointer), and add early argument validation.
  • Add a new API test covering zero-length IKM behavior (including crypto-callback visibility when enabled) and relevant BAD_FUNC_ARG edge cases.
  • Fix/clarify Doxygen documentation and examples for the TLS 1.3 HKDF Extract APIs (including syncing the Japanese translation).

Reviewed changes

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

Show a summary per file
File Description
wolfcrypt/src/kdf.c Avoids writing into caller ikm when ikmLen==0 by using a local zero buffer and adds argument validation before any writes.
tests/api/test_hmac.h Registers the new TLS 1.3 HKDF Extract zero-length IKM API test.
tests/api/test_hmac.c Adds a dedicated test ensuring ikmLen==0 works with ikm==NULL, doesn’t mutate caller buffers, and matches explicit-zero behavior.
doc/dox_comments/header_files/hmac.h Updates TLS 1.3 HKDF Extract documentation (argument semantics, examples, and behavior notes).
doc/dox_comments/header_files-ja/hmac.h Mirrors the documentation updates in the Japanese Doxygen headers.

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

Comment thread doc/dox_comments/header_files/hmac.h Outdated
Comment thread doc/dox_comments/header_files/hmac.h Outdated
Comment thread doc/dox_comments/header_files-ja/hmac.h Outdated
Comment thread doc/dox_comments/header_files-ja/hmac.h Outdated
@github-actions

github-actions Bot commented Jul 17, 2026

Copy link
Copy Markdown

MemBrowse Memory Report

gcc-arm-cortex-m4-pkcs7

  • FLASH: .text +64 B (+0.0%, 213,050 B / 262,144 B, total: 81% used)

gcc-arm-cortex-m4-rsa-only

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

gcc-arm-cortex-m7-tls13

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fenrir Automated Review — PR #10938

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

No new issues found in the changed files. ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants