Bound KDF response output size against received frame#467
Open
yosuke-wolfssl wants to merge 1 commit into
Open
Bound KDF response output size against received frame#467yosuke-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 client-side KDF response handling in wh_client_crypto.c by validating the declared output size (res->outSz) against the actual received frame length (res_len), preventing reads past the end of the received response when dealing with malformed or compromised server replies.
Changes:
- Add defensive
res_len-based bounds checks in_HkdfMakeKeyand_CmacKdfMakeKeybefore consuming/copying response output. - Apply the bounds check unconditionally (including cache/no-export paths).
- Add a new
test-refactormisc test that simulates truncated/overstated responses via a stub transport and assertsWH_ERROR_ABORTED+ unchanged caller output buffer.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| test-refactor/wh_test_list.c | Registers the new misc test in the refactored test suite. |
| test-refactor/misc/wh_test_client_respbounds.c | Adds a stub-transport regression test covering overstated outSz and truncated-frame cases for HKDF/CMAC-KDF. |
| src/wh_client_crypto.c | Enforces res->outSz ≤ bytes actually received (after headers) for HKDF/CMAC-KDF responses. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #467
Scan targets checked: wolfhsm-crypto-bugs, wolfhsm-src
No new issues found in the changed files. ✅
This was referenced Jul 22, 2026
Frauschi
approved these changes
Jul 23, 2026
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.
Problem
_HkdfMakeKeyand_CmacKdfMakeKeycopiedres->outSzbytes out of the commbuffer after checking the declared size only against the caller's output
buffer. Neither used
res_len, the number of bytes actually received, so amalformed or compromised server response could declare more output than the
frame carried. The client would then read past the end of the received
response and hand the caller data that was never derived key material. Every
other variable-length response handler in
wh_client_crypto.calreadyperforms this check (
wh_Client_EccSignResponse,_EccSharedSecretResponse,wh_Client_AesGcmResponse,wh_Client_MlKemDecapsulate, and others).With a conforming server the server-side
outSz > max_sizeguard preventsthis, so the impact is confined to malformed or compromised server responses,
matching the threat model of the other findings in this class.
Addressed by f_6201.
Changes:
res->outSzagainst the received frame length in_HkdfMakeKeyand
_CmacKdfMakeKey, before the response is consumed. Matches the existinghdr_szidiom used by the other response handlers in this file.cache path reports
outSz == 0, so a conforming response passes either way,and a truncated frame is malformed regardless of whether the caller asked
for output.
test-refactor/misc/wh_test_client_respbounds.c, registered inwh_test_list.cunder the misc group. A stub client transport stands in forthe server with no server process, echoing the request header and replying
with a frame that overstates the output size. Both KDF helpers are driven
through two malformed shapes: a full response header carrying no key
material, and a frame too short to hold the response header at all. Each
case asserts
WH_ERROR_ABORTEDand that the caller's buffer was leftuntouched.
Verification:
the fix the client returned
WH_ERROR_OKand copied 256 bytes of adjacentcomm-buffer contents into the caller's key buffer. With only the second
half of the guard the short-frame case still slipped through, since
res_len - hdr_szunderflows whenres_lenis smaller.test-refactorsuite: 38 passed, 0 failed. Legacytestsuite clean.