Skip to content

Commit 5a5f35b

Browse files
jackctj117jackctj117
authored
Fix coverity issues (#162)
coverity fixes
1 parent 5d842c7 commit 5a5f35b

File tree

7 files changed

+136
-54
lines changed

7 files changed

+136
-54
lines changed

‎src/wh_client_crypto.c‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2114,7 +2114,8 @@ int wh_Client_RsaFunction(whClientContext* ctx, RsaKey* key, int rsa_type,
21142114
if ((in != NULL) && (in_len > 0)) {
21152115
memcpy(req_in, in, in_len);
21162116
}
2117-
req->outLen = *inout_out_len;
2117+
/* Set output length only when provided to avoid NULL dereference */
2118+
req->outLen = (inout_out_len != NULL) ? *inout_out_len : 0;
21182119

21192120
/* Send Request */
21202121
ret = wh_Client_SendRequest(ctx, group, action, req_len,

‎src/wh_client_nvm.c‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -626,7 +626,7 @@ int wh_Client_NvmReadRequest(whClientContext* c,
626626
int wh_Client_NvmReadResponse(whClientContext* c, int32_t *out_rc,
627627
whNvmSize *out_len, uint8_t* data)
628628
{
629-
uint8_t buffer[WH_MESSAGE_NVM_MAX_READ_LEN] = {0};
629+
uint8_t buffer[WOLFHSM_CFG_COMM_DATA_LEN] = {0};
630630
whMessageNvm_ReadResponse* msg = (whMessageNvm_ReadResponse*)buffer;
631631
uint16_t hdr_len = sizeof(*msg);
632632
uint8_t* payload = buffer + hdr_len;
@@ -645,13 +645,14 @@ int wh_Client_NvmReadResponse(whClientContext* c, int32_t *out_rc,
645645
&resp_size, buffer);
646646
if (rc == 0) {
647647
/* Validate response */
648-
if ( (resp_group != WH_MESSAGE_GROUP_NVM) ||
649-
(resp_action != WH_MESSAGE_NVM_ACTION_READ) ||
650-
(resp_size < hdr_len) ||
651-
(resp_size - hdr_len > WH_MESSAGE_NVM_MAX_READ_LEN) ){
648+
if ((resp_group != WH_MESSAGE_GROUP_NVM) ||
649+
(resp_action != WH_MESSAGE_NVM_ACTION_READ) ||
650+
(resp_size < hdr_len) || (resp_size > sizeof(buffer)) ||
651+
(resp_size - hdr_len > WH_MESSAGE_NVM_MAX_READ_LEN)) {
652652
/* Invalid message */
653653
rc = WH_ERROR_ABORTED;
654-
} else {
654+
}
655+
else {
655656
/* Valid message */
656657
if (out_rc != NULL) {
657658
*out_rc = msg->rc;

‎src/wh_nvm_flash.c‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1190,9 +1190,13 @@ int wh_NvmFlash_DestroyObjects(void* c, whNvmId list_count,
11901190
/* Write each used object to new partition */
11911191
for (entry = 0; entry < WOLFHSM_CFG_NVM_OBJECT_COUNT; entry++) {
11921192
if (d->objects[entry].state.status == NF_STATUS_USED) {
1193-
/* TODO: Handle errors here better. Break out of loop? */
11941193
ret = nfObject_Copy(context, entry,
11951194
dest_part, &dest_object, &dest_data);
1195+
if (ret != WH_ERROR_OK) {
1196+
/* Abort reclaim to avoid activating a partially copied
1197+
* partition */
1198+
return ret;
1199+
}
11961200
}
11971201
}
11981202

‎src/wh_server_counter.c‎

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -56,16 +56,17 @@ int wh_Server_HandleCounter(whServerContext* server, uint16_t magic,
5656

5757
switch (action) {
5858
case WH_COUNTER_INIT: {
59-
whMessageCounter_InitRequest req;
60-
whMessageCounter_InitResponse resp;
59+
whMessageCounter_InitRequest req = {0};
60+
whMessageCounter_InitResponse resp = {0};
6161

6262
/* translate request */
6363
(void)wh_MessageCounter_TranslateInitRequest(
6464
magic, (whMessageCounter_InitRequest*)req_packet, &req);
6565

6666
/* write 0 to nvm with the supplied id and user_id */
6767
meta->id = WH_MAKE_KEYID(WH_KEYTYPE_COUNTER,
68-
server->comm->client_id, req.counterId);
68+
(uint16_t)server->comm->client_id,
69+
(uint16_t)req.counterId);
6970
/* use the label buffer to hold the counter value */
7071
*counter = req.counter;
7172
ret = wh_Nvm_AddObjectWithReclaim(server->nvm, meta, 0, NULL);
@@ -83,19 +84,20 @@ int wh_Server_HandleCounter(whServerContext* server, uint16_t magic,
8384
} break;
8485

8586
case WH_COUNTER_INCREMENT: {
86-
whMessageCounter_IncrementRequest req;
87-
whMessageCounter_IncrementResponse resp;
87+
whMessageCounter_IncrementRequest req = {0};
88+
whMessageCounter_IncrementResponse resp = {0};
8889

8990
/* translate request */
9091
(void)wh_MessageCounter_TranslateIncrementRequest(
9192
magic, (whMessageCounter_IncrementRequest*)req_packet, &req);
9293

9394
/* read the counter, stored in the metadata label */
94-
ret = wh_Nvm_GetMetadata(server->nvm,
95-
WH_MAKE_KEYID(WH_KEYTYPE_COUNTER,
96-
server->comm->client_id,
97-
req.counterId),
98-
meta);
95+
ret = wh_Nvm_GetMetadata(
96+
server->nvm,
97+
WH_MAKE_KEYID(WH_KEYTYPE_COUNTER,
98+
(uint16_t)server->comm->client_id,
99+
(uint16_t)req.counterId),
100+
meta);
99101
resp.rc = ret;
100102

101103
/* increment and write the counter back */
@@ -128,19 +130,20 @@ int wh_Server_HandleCounter(whServerContext* server, uint16_t magic,
128130
} break;
129131

130132
case WH_COUNTER_READ: {
131-
whMessageCounter_ReadRequest req;
132-
whMessageCounter_ReadResponse resp;
133+
whMessageCounter_ReadRequest req = {0};
134+
whMessageCounter_ReadResponse resp = {0};
133135

134136
/* translate request */
135137
(void)wh_MessageCounter_TranslateReadRequest(
136138
magic, (whMessageCounter_ReadRequest*)req_packet, &req);
137139

138140
/* read the counter, stored in the metadata label */
139-
ret = wh_Nvm_GetMetadata(server->nvm,
140-
WH_MAKE_KEYID(WH_KEYTYPE_COUNTER,
141-
server->comm->client_id,
142-
req.counterId),
143-
meta);
141+
ret = wh_Nvm_GetMetadata(
142+
server->nvm,
143+
WH_MAKE_KEYID(WH_KEYTYPE_COUNTER,
144+
(uint16_t)server->comm->client_id,
145+
(uint16_t)req.counterId),
146+
meta);
144147
resp.rc = ret;
145148

146149
/* return counter to the caller */
@@ -158,15 +161,16 @@ int wh_Server_HandleCounter(whServerContext* server, uint16_t magic,
158161
} break;
159162

160163
case WH_COUNTER_DESTROY: {
161-
whMessageCounter_DestroyRequest req;
162-
whMessageCounter_DestroyResponse resp;
164+
whMessageCounter_DestroyRequest req = {0};
165+
whMessageCounter_DestroyResponse resp = {0};
163166

164167
/* translate request */
165168
(void)wh_MessageCounter_TranslateDestroyRequest(
166169
magic, (whMessageCounter_DestroyRequest*)req_packet, &req);
167170

168171
counterId = WH_MAKE_KEYID(WH_KEYTYPE_COUNTER,
169-
server->comm->client_id, req.counterId);
172+
(uint16_t)server->comm->client_id,
173+
(uint16_t)req.counterId);
170174

171175
ret = wh_Nvm_DestroyObjects(server->nvm, 1, &counterId);
172176
resp.rc = ret;

‎src/wh_server_crypto.c‎

Lines changed: 84 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -302,16 +302,25 @@ static int _HandleRsaKeyGen(whServerContext* ctx, uint16_t magic,
302302
printf("[server] RsaKeyGen UniqueId: keyId:%u, ret:%d\n",
303303
key_id, ret);
304304
#endif
305+
if (ret != WH_ERROR_OK) {
306+
/* Early return on unique ID generation failure */
307+
wc_FreeRsaKey(rsa);
308+
return ret;
309+
}
305310
}
306311

307-
ret = wh_Server_CacheImportRsaKey(ctx, rsa, key_id, flags,
308-
label_size, label);
312+
if (ret == 0) {
313+
ret = wh_Server_CacheImportRsaKey(ctx, rsa, key_id, flags,
314+
label_size, label);
315+
}
309316
#ifdef DEBUG_CRYPTOCB_VERBOSE
310317
printf("[server] RsaKeyGen CacheKeyRsa: keyId:%u, ret:%d\n",
311318
key_id, ret);
312319
#endif
313-
res.keyId = WH_KEYID_ID(key_id);
314-
res.len = 0;
320+
if (ret == 0) {
321+
res.keyId = WH_KEYID_ID(key_id);
322+
res.len = 0;
323+
}
315324
}
316325
}
317326
wc_FreeRsaKey(rsa);
@@ -742,9 +751,16 @@ static int _HandleEccKeyGen(whServerContext* ctx, uint16_t magic,
742751
printf("[server] %s UniqueId: keyId:%u, ret:%d\n", __func__,
743752
key_id, ret);
744753
#endif
754+
if (ret != WH_ERROR_OK) {
755+
/* Early return on unique ID generation failure */
756+
wc_ecc_free(key);
757+
return ret;
758+
}
759+
}
760+
if (ret == 0) {
761+
ret = wh_Server_EccKeyCacheImport(ctx, key, key_id, flags,
762+
label_size, label);
745763
}
746-
ret = wh_Server_EccKeyCacheImport(ctx, key, key_id, flags,
747-
label_size, label);
748764
#ifdef DEBUG_CRYPTOCB
749765
printf("[server] %s CacheImport: keyId:%u, ret:%d\n", __func__,
750766
key_id, ret);
@@ -1159,10 +1175,17 @@ static int _HandleCurve25519KeyGen(whServerContext* ctx, uint16_t magic,
11591175
printf("[server] %s UniqueId: keyId:%u, ret:%d\n", __func__,
11601176
key_id, ret);
11611177
#endif
1178+
if (ret != WH_ERROR_OK) {
1179+
/* Early return on unique ID generation failure */
1180+
wc_curve25519_free(key);
1181+
return ret;
1182+
}
11621183
}
11631184

1164-
ret = wh_Server_CacheImportCurve25519Key(
1165-
ctx, key, key_id, flags, label_size, label);
1185+
if (ret == 0) {
1186+
ret = wh_Server_CacheImportCurve25519Key(
1187+
ctx, key, key_id, flags, label_size, label);
1188+
}
11661189
#ifdef DEBUG_CRYPTOCB
11671190
printf("[server] %s CacheImport: keyId:%u, ret:%d\n", __func__,
11681191
key_id, ret);
@@ -1928,12 +1951,16 @@ static int _HandleCmac(whServerContext* ctx, uint16_t magic, uint16_t seq,
19281951
if (moveToBigCache == 1) {
19291952
ret = wh_Server_KeystoreEvictKey(ctx, keyId);
19301953
}
1931-
meta->id = keyId;
1932-
meta->len = sizeof(ctx->crypto->algoCtx.cmac);
1933-
ret = wh_Server_KeystoreCacheKey(
1934-
ctx, meta, (uint8_t*)ctx->crypto->algoCtx.cmac);
1935-
res.keyId = WH_KEYID_ID(keyId);
1936-
res.outSz = 0;
1954+
if (ret == 0) {
1955+
meta->id = keyId;
1956+
meta->len = sizeof(ctx->crypto->algoCtx.cmac);
1957+
ret = wh_Server_KeystoreCacheKey(
1958+
ctx, meta, (uint8_t*)ctx->crypto->algoCtx.cmac);
1959+
if (ret == 0) {
1960+
res.keyId = WH_KEYID_ID(keyId);
1961+
res.outSz = 0;
1962+
}
1963+
}
19371964
#ifdef DEBUG_CRYPTOCB_VERBOSE
19381965
printf("[server] cmac saved state in keyid:%x %x len:%u ret:%d type:%d\n",
19391966
keyId, WH_KEYID_ID(keyId), meta->len, ret, ctx->crypto->algoCtx.cmac->type);
@@ -1969,7 +1996,8 @@ static int _HandleSha256(whServerContext* ctx, uint16_t magic,
19691996
int ret = 0;
19701997
wc_Sha256 sha256[1];
19711998
whMessageCrypto_Sha256Request req;
1972-
whMessageCrypto_Sha2Response res;
1999+
whMessageCrypto_Sha2Response res = {0};
2000+
19732001
/* Translate the request */
19742002
ret = wh_MessageCrypto_TranslateSha256Request(magic, cryptoDataIn, &req);
19752003
if (ret != 0) {
@@ -1986,6 +2014,10 @@ static int _HandleSha256(whServerContext* ctx, uint16_t magic,
19862014
sha256->hiLen = req.resumeState.hiLen;
19872015

19882016
if (req.isLastBlock) {
2017+
/* Validate lastBlockLen to prevent potential buffer overread */
2018+
if ((unsigned int)req.lastBlockLen > WC_SHA256_BLOCK_SIZE) {
2019+
return WH_ERROR_BADARGS;
2020+
}
19892021
/* wolfCrypt (or cryptoCb) is responsible for last block padding */
19902022
if (ret == 0) {
19912023
ret = wc_Sha256Update(sha256, req.inBlock, req.lastBlockLen);
@@ -2348,9 +2380,17 @@ static int _HandleMlDsaKeyGen(whServerContext* ctx, uint16_t magic,
23482380
printf("[server] %s UniqueId: keyId:%u, ret:%d\n",
23492381
__func__, key_id, ret);
23502382
#endif
2383+
if (ret != WH_ERROR_OK) {
2384+
/* Early return on unique ID generation failure
2385+
*/
2386+
wc_MlDsaKey_Free(key);
2387+
return ret;
2388+
}
2389+
}
2390+
if (ret == 0) {
2391+
ret = wh_Server_MlDsaKeyCacheImport(
2392+
ctx, key, key_id, flags, label_size, label);
23512393
}
2352-
ret = wh_Server_MlDsaKeyCacheImport(
2353-
ctx, key, key_id, flags, label_size, label);
23542394
#ifdef DEBUG_CRYPTOCB
23552395
printf("[server] %s CacheImport: keyId:%u, ret:%d\n",
23562396
__func__, key_id, ret);
@@ -2410,6 +2450,16 @@ static int _HandleMlDsaSign(whServerContext* ctx, uint16_t magic,
24102450
uint32_t options = req.options;
24112451
int evict = !!(options & WH_MESSAGE_CRYPTO_MLDSA_SIGN_OPTIONS_EVICT);
24122452

2453+
/* Validate input length against available data to prevent buffer overread
2454+
*/
2455+
if (inSize < sizeof(whMessageCrypto_MlDsaSignRequest)) {
2456+
return WH_ERROR_BADARGS;
2457+
}
2458+
word32 available_data = inSize - sizeof(whMessageCrypto_MlDsaSignRequest);
2459+
if (in_len > available_data) {
2460+
return WH_ERROR_BADARGS;
2461+
}
2462+
24132463
/* Response message */
24142464
byte* res_out =
24152465
(uint8_t*)(cryptoDataOut) + sizeof(whMessageCrypto_MlDsaSignResponse);
@@ -2479,6 +2529,17 @@ static int _HandleMlDsaVerify(whServerContext* ctx, uint16_t magic,
24792529
uint32_t sig_len = req.sigSz;
24802530
byte* req_sig =
24812531
(uint8_t*)(cryptoDataIn) + sizeof(whMessageCrypto_MlDsaVerifyRequest);
2532+
2533+
/* Validate lengths against available payload (overflow-safe) */
2534+
if (inSize < sizeof(whMessageCrypto_MlDsaVerifyRequest)) {
2535+
return WH_ERROR_BADARGS;
2536+
}
2537+
uint32_t available = inSize - sizeof(whMessageCrypto_MlDsaVerifyRequest);
2538+
if ((sig_len > available) || (hash_len > available) ||
2539+
(sig_len > (available - hash_len))) {
2540+
return WH_ERROR_BADARGS;
2541+
}
2542+
24822543
byte* req_hash = req_sig + sig_len;
24832544
int evict = !!(options & WH_MESSAGE_CRYPTO_MLDSA_VERIFY_OPTIONS_EVICT);
24842545

@@ -3463,6 +3524,12 @@ static int _HandleMlDsaKeyGenDma(whServerContext* ctx, uint16_t magic,
34633524
printf("[server] %s UniqueId: keyId:%u, ret:%d\n",
34643525
__func__, keyId, ret);
34653526
#endif
3527+
if (ret != WH_ERROR_OK) {
3528+
/* Early return on unique ID generation failure
3529+
*/
3530+
wc_MlDsaKey_Free(key);
3531+
return ret;
3532+
}
34663533
}
34673534

34683535
if (ret == 0) {

‎src/wh_server_keystore.c‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -448,7 +448,7 @@ int wh_Server_KeystoreReadKey(whServerContext* server, whKeyId keyId,
448448
}
449449
/* cache key if free slot, will only kick out other commited keys */
450450
if (ret == 0 && out != NULL) {
451-
wh_Server_KeystoreCacheKey(server, meta, out);
451+
(void)wh_Server_KeystoreCacheKey(server, meta, out);
452452
}
453453
#ifdef WOLFHSM_CFG_SHE_EXTENSION
454454
/* use empty key of zeros if we couldn't find the master ecu key */
@@ -690,7 +690,7 @@ int wh_Server_HandleKeyRequest(whServerContext* server, uint16_t magic,
690690

691691
case WH_KEY_EVICT: {
692692
whMessageKeystore_EvictRequest req;
693-
whMessageKeystore_EvictResponse resp;
693+
whMessageKeystore_EvictResponse resp = {0};
694694

695695
(void)wh_MessageKeystore_TranslateEvictRequest(
696696
magic, (whMessageKeystore_EvictRequest*)req_packet, &req);
@@ -702,9 +702,14 @@ int wh_Server_HandleKeyRequest(whServerContext* server, uint16_t magic,
702702
/* TODO: Are there any fatal server errors? */
703703
ret = WH_ERROR_OK;
704704

705-
(void)wh_MessageKeystore_TranslateEvictResponse(
706-
magic, &resp, (whMessageKeystore_EvictResponse*)resp_packet);
707-
*out_resp_size = sizeof(resp);
705+
if (ret == WH_ERROR_OK) {
706+
resp.ok = 0;
707+
708+
(void)wh_MessageKeystore_TranslateEvictResponse(
709+
magic, &resp,
710+
(whMessageKeystore_EvictResponse*)resp_packet);
711+
*out_resp_size = sizeof(resp);
712+
}
708713
} break;
709714

710715
case WH_KEY_EXPORT: {

0 commit comments

Comments
 (0)