Skip to content

Add checking in SHE response handlers#391

Open
padelsbach wants to merge 1 commit into
wolfSSL:mainfrom
padelsbach:she-response-checking
Open

Add checking in SHE response handlers#391
padelsbach wants to merge 1 commit into
wolfSSL:mainfrom
padelsbach:she-response-checking

Conversation

@padelsbach
Copy link
Copy Markdown
Contributor

Check for group, action and len in SHE response handlers. Found with experimental fuzzing.

Comment thread src/wh_client_she.c

resp = (whMessageShe_SetUidResponse*)wh_CommClient_GetDataPtr(c->comm);
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)resp);
if (ret == WH_ERROR_OK) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Could macro'ize this, but not sure if that's more readable

@padelsbach padelsbach marked this pull request as ready for review May 29, 2026 23:07
Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #391

Scan targets checked: wolfhsm-core-bugs, wolfhsm-src

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

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

Comment thread src/wh_client_she.c
if (group != WH_MESSAGE_GROUP_SHE ||
action != WH_SHE_ENC_ECB ||
dataSz < sizeof(*resp) ||
dataSz < sizeof(*resp) + resp->sz) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 [Low] Payload length checked before response error · Incorrect error handling

The variable-length response handlers validate resp->sz before resp->rc, so a header-only SHE error response with a nonzero size is returned as WH_ERROR_ABORTED instead of the SHE error.

Fix: Check only the fixed header first, read resp->rc, and validate resp->sz only when resp->rc == WH_ERROR_OK.

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