Skip to content

Add data len check in wh_CommServer_RecvRequest #389

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
padelsbach wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add data len check in wh_CommServer_RecvRequest #389

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 14 additions & 9 deletions src/wh_comm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,14 +350,14 @@ int wh_CommServer_Init(whCommServer* context, const whCommServerConfig* config,

int wh_CommServer_RecvRequest(whCommServer* context,
uint16_t* out_magic, uint16_t* out_kind, uint16_t* out_seq,
uint16_t* out_size, void* data)
uint16_t* out_size, uint16_t data_size, void* data)
{
int rc = 0;
uint16_t magic = 0;
uint16_t kind = 0;
uint16_t seq = 0;
uint16_t size = sizeof(context->packet);
uint16_t data_size = 0;
uint16_t req_size = 0;

if ((context == NULL) || (context->hdr == NULL) ||
(context->initialized == 0) || (context->transport_cb == NULL) ||
Expand All @@ -373,21 +373,26 @@ int wh_CommServer_RecvRequest(whCommServer* context,
rc = WH_ERROR_ABORTED;
}
if (rc == 0) {
data_size = size - sizeof(*context->hdr);
req_size = size - sizeof(*context->hdr);
magic = context->hdr->magic;
kind = wh_Translate16(magic, context->hdr->kind);
seq = wh_Translate16(magic, context->hdr->seq);

/* Copy the data from the internal buffer if necessary */
if ( (data != NULL) &&
(data_size != 0) &&
(data != context->data) ) {
memcpy(data, context->data, data_size);
if ((data != NULL) && (req_size > data_size)) {
rc = WH_ERROR_BUFFER_SIZE;
}
else {
/* Copy the data from the internal buffer if necessary */
if ( (data != NULL) &&
(req_size != 0) &&
(data != context->data) ) {
memcpy(data, context->data, req_size);
}
}
if (out_magic != NULL) *out_magic = magic;
if (out_kind != NULL) *out_kind = kind;
if (out_seq != NULL) *out_seq = seq;
if (out_size != NULL) *out_size = data_size;
if (out_size != NULL) *out_size = req_size;
}
}
return rc;
Expand Down
2 changes: 1 addition & 1 deletion src/wh_server.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,7 @@ int wh_Server_HandleRequestMessage(whServerContext* server)
}

int rc = wh_CommServer_RecvRequest(server->comm, &magic, &kind, &seq,
&size, data);
&size, WOLFHSM_CFG_COMM_DATA_LEN, data);
/* Got a packet? */
if (rc == WH_ERROR_OK) {
group = WH_MESSAGE_GROUP(kind);
Expand Down
12 changes: 8 additions & 4 deletions test-refactor/misc/wh_test_comm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,8 @@ static int _whTest_CommMem(void)
WH_TEST_ASSERT_RETURN(WH_ERROR_NOTREADY ==
wh_CommServer_RecvRequest(server, &rx_req_flags,
&rx_req_type, &rx_req_seq,
&rx_req_len, rx_req));
&rx_req_len,
sizeof(rx_req), rx_req));

/* RecvResponse with no outstanding request short-circuits to NOTREADY
* without touching the transport. */
Expand Down Expand Up @@ -161,7 +162,8 @@ static int _whTest_CommMem(void)

WH_TEST_RETURN_ON_FAIL(
wh_CommServer_RecvRequest(server, &rx_req_flags, &rx_req_type,
&rx_req_seq, &rx_req_len, rx_req));
&rx_req_seq, &rx_req_len,
sizeof(rx_req), rx_req));

WH_TEST_DEBUG_PRINT("Server RecvRequest:%d, flags %x, type:%x, seq:%d, len:%d, %s\n",
ret, rx_req_flags, rx_req_type, rx_req_seq, rx_req_len, rx_req);
Expand Down Expand Up @@ -210,7 +212,8 @@ static int _whTest_CommMem(void)

/* Server completes the exchange */
WH_TEST_RETURN_ON_FAIL(wh_CommServer_RecvRequest(
server, &rx_req_flags, &rx_req_type, &rx_req_seq, &rx_req_len, rx_req));
server, &rx_req_flags, &rx_req_type, &rx_req_seq, &rx_req_len,
sizeof(rx_req), rx_req));
(void)snprintf((char*)tx_resp, sizeof(tx_resp), "Resp");
tx_resp_len = (uint16_t)strlen((char*)tx_resp);
WH_TEST_RETURN_ON_FAIL(wh_CommServer_SendResponse(
Expand Down Expand Up @@ -240,7 +243,8 @@ static int _whTest_CommMem(void)
/* Drain the abandoned exchange on the server side so the transport state
* doesn't linger across tests. */
WH_TEST_RETURN_ON_FAIL(wh_CommServer_RecvRequest(
server, &rx_req_flags, &rx_req_type, &rx_req_seq, &rx_req_len, rx_req));
server, &rx_req_flags, &rx_req_type, &rx_req_seq, &rx_req_len,
sizeof(rx_req), rx_req));
WH_TEST_RETURN_ON_FAIL(wh_CommServer_SendResponse(
server, rx_req_flags, rx_req_type, rx_req_seq, tx_resp_len, tx_resp));

Expand Down
14 changes: 9 additions & 5 deletions test/wh_test_comm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,8 @@ int whTest_CommMem(void)
WH_TEST_ASSERT_RETURN(WH_ERROR_NOTREADY ==
wh_CommServer_RecvRequest(server, &rx_req_flags,
&rx_req_type, &rx_req_seq,
&rx_req_len, rx_req));
&rx_req_len,
sizeof(rx_req), rx_req));

/* RecvResponse with no outstanding request short-circuits to NOTREADY
* without touching the transport. */
Expand Down Expand Up @@ -176,7 +177,8 @@ int whTest_CommMem(void)

WH_TEST_RETURN_ON_FAIL(
wh_CommServer_RecvRequest(server, &rx_req_flags, &rx_req_type,
&rx_req_seq, &rx_req_len, rx_req));
&rx_req_seq, &rx_req_len,
sizeof(rx_req), rx_req));

WH_TEST_DEBUG_PRINT("Server RecvRequest:%d, flags %x, type:%x, seq:%d, len:%d, %s\n",
ret, rx_req_flags, rx_req_type, rx_req_seq, rx_req_len, rx_req);
Expand Down Expand Up @@ -225,7 +227,8 @@ int whTest_CommMem(void)

/* Server completes the exchange */
WH_TEST_RETURN_ON_FAIL(wh_CommServer_RecvRequest(
server, &rx_req_flags, &rx_req_type, &rx_req_seq, &rx_req_len, rx_req));
server, &rx_req_flags, &rx_req_type, &rx_req_seq, &rx_req_len,
sizeof(rx_req), rx_req));
(void)snprintf((char*)tx_resp, sizeof(tx_resp), "Resp");
tx_resp_len = (uint16_t)strlen((char*)tx_resp);
WH_TEST_RETURN_ON_FAIL(wh_CommServer_SendResponse(
Expand Down Expand Up @@ -255,7 +258,8 @@ int whTest_CommMem(void)
/* Drain the abandoned exchange on the server side so the transport state
* doesn't linger across tests. */
WH_TEST_RETURN_ON_FAIL(wh_CommServer_RecvRequest(
server, &rx_req_flags, &rx_req_type, &rx_req_seq, &rx_req_len, rx_req));
server, &rx_req_flags, &rx_req_type, &rx_req_seq, &rx_req_len,
sizeof(rx_req), rx_req));
WH_TEST_RETURN_ON_FAIL(wh_CommServer_SendResponse(
server, rx_req_flags, rx_req_type, rx_req_seq, tx_resp_len, tx_resp));

Expand Down Expand Up @@ -379,7 +383,7 @@ static void* _whCommServerTask(void* cf)
do {
ret = wh_CommServer_RecvRequest(server, &rx_req_flags, &rx_req_type,
&rx_req_seq, &rx_req_len,
rx_req);
sizeof(rx_req), rx_req);

WH_TEST_ASSERT_MSG((ret == WH_ERROR_NOTREADY) || (0 == ret),
"Server RecvRequest: ret=%d", ret);
Expand Down
7 changes: 5 additions & 2 deletions wolfhsm/wh_comm.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,11 +309,14 @@ int wh_CommServer_Init(whCommServer* context, const whCommServerConfig* config,
whCommSetConnectedCb connectcb, void* connectcb_arg);

/* If a request packet has been buffered, get the header and copy the data out
* of the buffer.
* of the buffer. data_size is the capacity of the caller-supplied data buffer;
* if the received payload exceeds it, returns WH_ERROR_BUFFER_SIZE with
* *out_size set to the required size. On success *out_size holds the actual
* payload size.
*/
int wh_CommServer_RecvRequest(whCommServer* context,
uint16_t* out_magic, uint16_t* out_kind, uint16_t* out_seq,
uint16_t* out_size, void* data);
uint16_t* out_size, uint16_t data_size, void* data);

/* Upon completion of the request, send the response packet using the same seq
* as the incoming request. Note that overriding the seq number should only be
Expand Down
Loading