Skip to content

Commit 78b9b72

Browse files
committed
Fix uint16_t truncation of req_len in MlDsa, Ed25519, and Hkdf client functions by using uint32_t for intermediate length computation before bounds check
1 parent 18c46af commit 78b9b72

2 files changed

Lines changed: 62 additions & 15 deletions

File tree

bug-fix.txt

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
Hello Claude this file is to go over how to fix proposed bugs for this branch.
2+
3+
to start off read the STEPS section then go to the BUG-REPORT section to see the report is
4+
5+
STEPS:
6+
1. review proposued bug and confirm the existance
7+
2. If bug does exisit
8+
a. come up with option for the fix and compare it to proposed fix if there is one
9+
b. Check all docs and commit history for effeted code to see if it's intentinal or the so called bug is applicatible in certaion scenarioes
10+
3. Once the bug fix is found
11+
a. Analyze to see if there are any other implications in other spots in the code base and if any other changes need to be made to compinsate for the fix.
12+
b. analayze if there should be a test in place to prevent this from happneing in the future
13+
14+
15+
BUG-REPORT:
16+
# 776: uint16_t Truncation of Request Length in wh_Client_MlDsaSign and wh_Client_MlDsaVerify
17+
18+
- **Severity:** Medium
19+
- **Status:** open
20+
- **File:** `src/wh_client_crypto.c`
21+
- **Function:** `wh_Client_MlDsaSign, wh_Client_MlDsaVerify`
22+
- **Category:** integer_type_conversion
23+
- **Discovered:** 2026-03-15 04:57:52
24+
25+
## Description
26+
27+
Both `wh_Client_MlDsaSign` and `wh_Client_MlDsaVerify` compute `req_len` as `uint16_t` from a sum involving `word32` (uint32_t) operands (`in_len`, `sig_len`, `msg_len`). If the sum exceeds 65535, the value silently wraps. The truncated `req_len` is then compared against `WOLFHSM_CFG_COMM_DATA_LEN` for bounds checking; a wrapped value could pass this check despite the actual data being much larger. Other functions in the same file (e.g., `wh_Client_CmacKdf` at line 3886) correctly use `uint32_t` for the intermediate computation and only cast to `uint16_t` after validating the bounds.
28+
29+
## Code
30+
31+
```c
32+
uint16_t req_len = sizeof(whMessageCrypto_GenericRequestHeader) +
33+
sizeof(*req) + sig_len + msg_len;
34+
```
35+
36+
## Recommendation
37+
38+
Use `uint32_t` for the intermediate length computation, validate it against `WOLFHSM_CFG_COMM_DATA_LEN`, then assign to `uint16_t` only after the bounds check passes. Follow the pattern at line 3886-3892 in `wh_Client_CmacKdf`.
39+

src/wh_client_crypto.c

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2704,11 +2704,12 @@ int wh_Client_Ed25519Sign(whClientContext* ctx, ed25519_key* key,
27042704
return WH_ERROR_BADARGS;
27052705
}
27062706

2707-
uint16_t req_len = sizeof(whMessageCrypto_GenericRequestHeader) +
2708-
sizeof(*req) + msgLen + contextLen;
2709-
if (req_len > WOLFHSM_CFG_COMM_DATA_LEN) {
2707+
uint32_t total_len = sizeof(whMessageCrypto_GenericRequestHeader) +
2708+
sizeof(*req) + msgLen + contextLen;
2709+
if (total_len > WOLFHSM_CFG_COMM_DATA_LEN) {
27102710
return WH_ERROR_BADARGS;
27112711
}
2712+
uint16_t req_len = (uint16_t)total_len;
27122713

27132714
if (WH_KEYID_ISERASED(key_id)) {
27142715
uint8_t keyLabel[] = "TempEd25519Sign";
@@ -2820,9 +2821,9 @@ int wh_Client_Ed25519Verify(whClientContext* ctx, ed25519_key* key,
28202821
whMessageCrypto_Ed25519VerifyResponse* res = NULL;
28212822
uint8_t* dataPtr = NULL;
28222823
whKeyId key_id = WH_DEVCTX_TO_KEYID(key->devCtx);
2823-
int evict = 0;
2824-
uint16_t req_len = sizeof(whMessageCrypto_GenericRequestHeader) +
2825-
sizeof(*req) + sigLen + msgLen + contextLen;
2824+
int evict = 0;
2825+
uint32_t total_len = sizeof(whMessageCrypto_GenericRequestHeader) +
2826+
sizeof(*req) + sigLen + msgLen + contextLen;
28262827

28272828
if ((ctx == NULL) || (key == NULL) || (sig == NULL) || (msg == NULL) ||
28282829
(out_res == NULL) || ((context == NULL) && (contextLen > 0))) {
@@ -2840,9 +2841,10 @@ int wh_Client_Ed25519Verify(whClientContext* ctx, ed25519_key* key,
28402841
return WH_ERROR_BADARGS;
28412842
}
28422843

2843-
if (req_len > WOLFHSM_CFG_COMM_DATA_LEN) {
2844+
if (total_len > WOLFHSM_CFG_COMM_DATA_LEN) {
28442845
return WH_ERROR_BADARGS;
28452846
}
2847+
uint16_t req_len = (uint16_t)total_len;
28462848

28472849
*out_res = 0;
28482850

@@ -3712,8 +3714,12 @@ static int _HkdfMakeKey(whClientContext* ctx, int hashType, whKeyId keyIdIn,
37123714
dataPtr, WC_ALGO_TYPE_KDF, WC_KDF_TYPE_HKDF, ctx->cryptoAffinity);
37133715

37143716
/* Calculate request length including variable-length data */
3715-
uint16_t req_len = sizeof(whMessageCrypto_GenericRequestHeader) +
3716-
sizeof(*req) + inKeySz + saltSz + infoSz;
3717+
uint32_t total_len = sizeof(whMessageCrypto_GenericRequestHeader) +
3718+
sizeof(*req) + inKeySz + saltSz + infoSz;
3719+
if (total_len > WOLFHSM_CFG_COMM_DATA_LEN) {
3720+
return WH_ERROR_BADARGS;
3721+
}
3722+
uint16_t req_len = (uint16_t)total_len;
37173723

37183724
/* Use the supplied key id if provided */
37193725
if (inout_key_id != NULL) {
@@ -5710,8 +5716,8 @@ int wh_Client_MlDsaSign(whClientContext* ctx, const byte* in, word32 in_len,
57105716
uint16_t group = WH_MESSAGE_GROUP_CRYPTO;
57115717
uint16_t action = WC_ALGO_TYPE_PK;
57125718

5713-
uint16_t req_len = sizeof(whMessageCrypto_GenericRequestHeader) +
5714-
sizeof(*req) + in_len + contextLen;
5719+
uint32_t total_len = sizeof(whMessageCrypto_GenericRequestHeader) +
5720+
sizeof(*req) + in_len + contextLen;
57155721
uint32_t options = 0;
57165722

57175723
/* Get data pointer from the context to use as request/response storage
@@ -5727,7 +5733,8 @@ int wh_Client_MlDsaSign(whClientContext* ctx, const byte* in, word32 in_len,
57275733
dataPtr, WC_PK_TYPE_PQC_SIG_SIGN, WC_PQC_SIG_TYPE_DILITHIUM,
57285734
ctx->cryptoAffinity);
57295735

5730-
if (req_len <= WOLFHSM_CFG_COMM_DATA_LEN) {
5736+
if (total_len <= WOLFHSM_CFG_COMM_DATA_LEN) {
5737+
uint16_t req_len = (uint16_t)total_len;
57315738
uint8_t* req_data = (uint8_t*)(req + 1);
57325739
if (evict != 0) {
57335740
options |= WH_MESSAGE_CRYPTO_MLDSA_SIGN_OPTIONS_EVICT;
@@ -5845,8 +5852,8 @@ int wh_Client_MlDsaVerify(whClientContext* ctx, const byte* sig, word32 sig_len,
58455852
uint16_t action = WC_ALGO_TYPE_PK;
58465853
uint32_t options = 0;
58475854

5848-
uint16_t req_len = sizeof(whMessageCrypto_GenericRequestHeader) +
5849-
sizeof(*req) + sig_len + msg_len + contextLen;
5855+
uint32_t total_len = sizeof(whMessageCrypto_GenericRequestHeader) +
5856+
sizeof(*req) + sig_len + msg_len + contextLen;
58505857

58515858

58525859
/* Get data pointer from the context to use as request/response storage
@@ -5862,7 +5869,8 @@ int wh_Client_MlDsaVerify(whClientContext* ctx, const byte* sig, word32 sig_len,
58625869
WC_PQC_SIG_TYPE_DILITHIUM,
58635870
ctx->cryptoAffinity);
58645871

5865-
if (req_len <= WOLFHSM_CFG_COMM_DATA_LEN) {
5872+
if (total_len <= WOLFHSM_CFG_COMM_DATA_LEN) {
5873+
uint16_t req_len = (uint16_t)total_len;
58665874
uint8_t* req_sig = (uint8_t*)(req + 1);
58675875
uint8_t* req_hash = req_sig + sig_len;
58685876

0 commit comments

Comments
 (0)