Add checking in SHE response handlers#391
Conversation
|
|
||
| resp = (whMessageShe_SetUidResponse*)wh_CommClient_GetDataPtr(c->comm); | ||
| ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)resp); | ||
| if (ret == WH_ERROR_OK) { |
There was a problem hiding this comment.
Could macro'ize this, but not sure if that's more readable
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
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.
| if (group != WH_MESSAGE_GROUP_SHE || | ||
| action != WH_SHE_ENC_ECB || | ||
| dataSz < sizeof(*resp) || | ||
| dataSz < sizeof(*resp) + resp->sz) { |
There was a problem hiding this comment.
🔵 [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.
Check for group, action and len in SHE response handlers. Found with experimental fuzzing.