Skip to content

Apply checking of req_size in handlers#299

Open
padelsbach wants to merge 1 commit intowolfSSL:mainfrom
padelsbach:padelsbach/ret-size-checking
Open

Apply checking of req_size in handlers#299
padelsbach wants to merge 1 commit intowolfSSL:mainfrom
padelsbach:padelsbach/ret-size-checking

Conversation

@padelsbach
Copy link
Contributor

@padelsbach padelsbach commented Mar 13, 2026

Several functions in wh_server_she.c ignored the received payload length with (void)req_size;. This change adds size checking and associated unit tests.

@padelsbach padelsbach force-pushed the padelsbach/ret-size-checking branch 2 times, most recently from 2e741ac to fc0e2e2 Compare March 16, 2026 20:29
@padelsbach padelsbach marked this pull request as ready for review March 17, 2026 19:49
@padelsbach padelsbach requested a review from bigbrett March 17, 2026 19:49
(void)wh_MessageShe_TranslateSetUidRequest(
magic, (whMessageShe_SetUidRequest*)req_packet, &req);
if (req_size < sizeof(req)) {
ret = WH_ERROR_BUFFER_SIZE;
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this leaks a non-SHE error code. This module can only return SHE errors. We have an error code translation function _TranslateSheReturnCode(). Check the rest of the code for similar issues plz.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Line 220 sets resp.rc = ret unlike the other functions. This is a pre-existing bug. Will fix

@padelsbach padelsbach force-pushed the padelsbach/ret-size-checking branch from fc0e2e2 to c0ee8d4 Compare March 18, 2026 00:09
resp.rc = ret;
resp.rc = _TranslateSheReturnCode(ret);
(void)wh_MessageShe_TranslateSetUidResponse(magic, &resp, resp_packet);
*out_resp_size = sizeof(resp);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was also a pre-existing issue. out_resp_size was not set by this handler

return WH_ERROR_BADARGS;
}

/* Use out_resp_size as the indicator of whether a response was generated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change ensures the response is sent, even if a handler found an issue. Credit to Claude for this.

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.

2 participants