From 8ba18cdd83b93678412387b0d64f73d28a4dedbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Tue, 16 Jun 2026 10:54:12 +0200 Subject: [PATCH] transport/comm: bound peer-controlled lengths on receive paths The shared-memory transport copied a peer-controlled length (req/resp CSR len, up to 65535) into the destination buffer without bounding it against the buffer's capacity, allowing an oversized value to overflow context->packet. - wh_transport_mem.c: clamp the recv copy to the caller-supplied destination capacity (*out_len) in both RecvRequest and RecvResponse, and reject a non-NULL data buffer with a NULL out_len. - wh_comm.c: the transport still reports the true length, so reject size > sizeof(context->packet) in both CommClient_RecvResponse and CommServer_RecvRequest before deriving data_size/req_size, preventing an over-read of context->packet on a truncated message. --- src/wh_comm.c | 14 ++++++++++---- src/wh_transport_mem.c | 28 ++++++++++++++++++++++++---- 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/src/wh_comm.c b/src/wh_comm.c index 06ad5ed1c..74600a674 100644 --- a/src/wh_comm.c +++ b/src/wh_comm.c @@ -183,9 +183,12 @@ int wh_CommClient_RecvResponse(whCommClient* context, #ifdef WOLFHSM_CFG_ENABLE_TIMEOUT (void)wh_Timeout_Stop(&context->respTimeout); #endif - if (size < sizeof(*context->hdr)) { - /* Size is too small - transport-level corruption; treat as fatal - * and clear pending since the caller must tear down anyway. */ + if ((size < sizeof(*context->hdr)) || + (size > sizeof(context->packet))) { + /* Size out of range - transport-level corruption (the transport + * clamps its copy, but reports the true peer-controlled length, so + * an oversized value here means a truncated/bogus message). Treat as + * fatal and clear pending since the caller must tear down anyway. */ context->pending = 0; rc = WH_ERROR_ABORTED; } @@ -369,7 +372,10 @@ int wh_CommServer_RecvRequest(whCommServer* context, &size, context->packet); if (rc == 0) { - if (size < sizeof(*context->hdr)) { + /* size is the true peer-controlled length; the transport clamps its own + * copy, so an out-of-range value here is transport-level corruption. */ + if ((size < sizeof(*context->hdr)) || + (size > sizeof(context->packet))) { rc = WH_ERROR_ABORTED; } if (rc == 0) { diff --git a/src/wh_transport_mem.c b/src/wh_transport_mem.c index 92e74d4c2..6478d7aef 100644 --- a/src/wh_transport_mem.c +++ b/src/wh_transport_mem.c @@ -150,8 +150,11 @@ int wh_TransportMem_RecvResponse(void* c, uint16_t* out_len, void* data) whTransportMemCsr req; whTransportMemCsr resp; + /* A destination without a capacity (out_len) can't be clamped, so reject + * it rather than copy a peer-controlled length unbounded. */ if ( (context == NULL) || - (context->initialized == 0)) { + (context->initialized == 0) || + ((data != NULL) && (out_len == NULL))) { return WH_ERROR_BADARGS; } @@ -170,7 +173,13 @@ int wh_TransportMem_RecvResponse(void* c, uint16_t* out_len, void* data) } if ((data != NULL) && (resp.s.len != 0)) { - wh_Utils_memcpy_invalidate(data, context->resp_data, resp.s.len); + /* Clamp to the destination capacity so a corrupt/oversized resp.s.len + * can't overflow `data`. The true length is still reported below. */ + uint16_t copy_len = resp.s.len; + if (copy_len > *out_len) { + copy_len = *out_len; + } + wh_Utils_memcpy_invalidate(data, context->resp_data, copy_len); } if (out_len != NULL) { @@ -234,8 +243,11 @@ int wh_TransportMem_RecvRequest(void* c, uint16_t* out_len, void* data) whTransportMemCsr req; whTransportMemCsr resp; + /* A destination without a capacity (out_len) can't be clamped, so reject + * it rather than copy a peer-controlled length unbounded. */ if ( (context == NULL) || - (context->initialized == 0)) { + (context->initialized == 0) || + ((data != NULL) && (out_len == NULL))) { return WH_ERROR_BADARGS; } @@ -254,7 +266,15 @@ int wh_TransportMem_RecvRequest(void* c, uint16_t* out_len, void* data) } if ((data != NULL) && (req.s.len != 0)) { - wh_Utils_memcpy_invalidate(data, context->req_data, req.s.len); + /* req.s.len is peer-controlled. Clamp to the destination capacity + * so an oversized len can't overflow `data`. The true length is still + * reported below, so the comm layer rejects an over-length request + * rather than acting on a truncation. */ + uint16_t copy_len = req.s.len; + if (copy_len > *out_len) { + copy_len = *out_len; + } + wh_Utils_memcpy_invalidate(data, context->req_data, copy_len); } if (out_len != NULL) { *out_len = req.s.len;