Avoid writing into caller ikm buffer in wc_Tls13_HKDF_Extract#10938
Open
yosuke-wolfssl wants to merge 1 commit into
Open
Avoid writing into caller ikm buffer in wc_Tls13_HKDF_Extract#10938yosuke-wolfssl wants to merge 1 commit into
yosuke-wolfssl wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
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 whenikmLen == 0(instead of writing through the caller’sikmpointer), 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.
|
yosuke-wolfssl
force-pushed
the
fix/f_6767
branch
from
July 17, 2026 03:54
148ef5b to
9c78cdc
Compare
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
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. ✅
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
wc_Tls13_HKDF_Extract()andwc_Tls13_HKDF_Extract_ex()write digest length zero bytesinto the caller supplied
ikmbuffer whenikmLen == 0, before any validation:The size this requires is not implied by the length the caller passes, which is
0, so itis an undocumented precondition. Saying "no input keying material" the natural way,
wc_Tls13_HKDF_Extract(prk, salt, saltLen, NULL, 0, WC_SHA256), runsXMEMSET(NULL, 0, 32)and crashes. A non NULL buffer sized for the
0length argument getslenbytes writtenpast the end. Both were reproduced under ASan as a SEGV on a NULL write and a
stack-buffer-overflowofWRITE 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 existingtest_wc_HKDF_NullKeyEdgeCasesasserts a NULL key of length0is 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.cpass buffers of 64, 48 orENCRYPT_LENbytes against a 32 or 48 byte digest, so this is an API boundary hazard forexternal callers.
Addressed by f_6767.
Changes
wolfcrypt/src/kdf.c: whenikmLen == 0, point aconst byte* localIkmalias at a localzeroed
tmpbuffer instead of writing through the caller's pointer, mirroring thetmp/localSaltidiomwc_HKDF_Extract_ex()already uses for a NULL salt. Validateprkandikmat entry.saltis deliberately left alone, sincewc_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
ikmbuffer whenikmLen == 0. The derived PRK is bit identical, since both old and new code feed HKDFlenzero bytes and only the source buffer changed. No in tree caller depends on the side effect.
tests/api/test_hmac.c: addstest_wc_Tls13_HKDF_Extract_ZeroLenIkm, which fails before thefix. A NULL
ikmwithikmLen == 0must succeed and derive the same PRK as an explicitzero buffer; a digest length sentinel passed with
ikmLen == 0must come back untouched.Covers both the wrapper and the
_exentry pointsrc/tls13.ccalls, every digest theswitch accepts, the
BAD_FUNC_ARGcases, and underWOLF_CRYPTO_CBthat a callbackreceives the substituted zeroed IKM rather than the caller's buffer.
Doxygen:
ikmwas documented as "pointer to putput for keying material", describing it as anoutput. Corrected, and documented that
ikmLen == 0substitutes a digest length zeroed IKMper 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 parameterfunction with
0landing onikm. Japanese translations kept in sync.Testing
testwolfcryptand fullunit.testpass with--enable-tls13 --enable-hkdf, with--enable-cryptocb, under ASan and UBSan, andNO_HASH_WRAPPERcompiles. The TLS 1.3 ciphersuite tests exercise the real
DeriveEarlySecretandDeriveMasterSecretzero lengthcallers and are unaffected. Also compile checked on the FIPS and selftest
#elsepath andthe
WOLFSSL_DEBUG_TLSblock.Note for reviewers: FIPS module boundary
wolfcrypt/src/kdf.cis inside the FIPS module boundary. It setsFIPS_NO_WRAPPERSandplaces code in the
.fipsA$h/.fipsB$hsegments underFIPS_VERSION3_GE(5,0,0), andcarries a
wolfCrypt_FIPS_KDF_sanityhook. The frozen 5.2.1 and 5.2.4 bundles ship their owncopy of
kdf.c, in segment.fipsA$m, which still contains the unfixed write. This fixtherefore does not reach FIPS users until it folds into a module revision, which is why the
new test skips FIPS and selftest builds.