From c2b5ea0be961a41a10148cc0cab87345454a541d Mon Sep 17 00:00:00 2001 From: Paul Adelsbach Date: Fri, 29 May 2026 15:46:20 -0700 Subject: [PATCH] Add data len check in wh_CommServer_RecvRequest --- src/wh_comm.c | 23 ++++++++++++++--------- src/wh_server.c | 2 +- test-refactor/misc/wh_test_comm.c | 12 ++++++++---- test/wh_test_comm.c | 14 +++++++++----- wolfhsm/wh_comm.h | 7 +++++-- 5 files changed, 37 insertions(+), 21 deletions(-) diff --git a/src/wh_comm.c b/src/wh_comm.c index 021df57a1..06ad5ed1c 100644 --- a/src/wh_comm.c +++ b/src/wh_comm.c @@ -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) || @@ -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; diff --git a/src/wh_server.c b/src/wh_server.c index 2618d29b7..c2c4b1255 100644 --- a/src/wh_server.c +++ b/src/wh_server.c @@ -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); diff --git a/test-refactor/misc/wh_test_comm.c b/test-refactor/misc/wh_test_comm.c index 0010c1939..2655f7cd4 100644 --- a/test-refactor/misc/wh_test_comm.c +++ b/test-refactor/misc/wh_test_comm.c @@ -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. */ @@ -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); @@ -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( @@ -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)); diff --git a/test/wh_test_comm.c b/test/wh_test_comm.c index b695569b3..42503a3b1 100644 --- a/test/wh_test_comm.c +++ b/test/wh_test_comm.c @@ -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. */ @@ -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); @@ -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( @@ -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)); @@ -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); diff --git a/wolfhsm/wh_comm.h b/wolfhsm/wh_comm.h index 2c9355d93..ab151b515 100644 --- a/wolfhsm/wh_comm.h +++ b/wolfhsm/wh_comm.h @@ -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