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;