Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
a20d5d3
Intialized req and resp as well as potenial int overflow issues
jackctj117 Sep 3, 2025
e089fe2
Initalized req resp and int overflow fixes for server_counter
jackctj117 Sep 3, 2025
7284ba4
Intialized req and resp as well as potenial int overflow issues
jackctj117 Sep 3, 2025
bf8bb1d
Initalized req resp and int overflow fixes for server_counter
jackctj117 Sep 3, 2025
6557562
Fix for 572974 Uninitialized scalar variable
jackctj117 Sep 9, 2025
d1e19be
Fixes for unused values
jackctj117 Sep 9, 2025
fd24a81
Merge branch 'coverity-fixes' of github.com:jackctj117/wolfHSM into c…
jackctj117 Sep 9, 2025
1e10397
rebase fix for server crypto3
jackctj117 Sep 11, 2025
46f9323
fix default wolfHSM path capitalization and relative path
bigbrett Sep 11, 2025
03c7345
rebase fix for server crypto4
jackctj117 Sep 11, 2025
1013d9c
Fixes for unused values
jackctj117 Sep 9, 2025
312d1bc
rebase fix
jackctj117 Sep 11, 2025
bf8c26f
rebase fix for server crypto
jackctj117 Sep 11, 2025
f738840
Fixes for unused values
jackctj117 Sep 9, 2025
7f158da
Support sha2-224, 384 and 512 (#144)
miyazakh Sep 11, 2025
2b381b1
Fix for 572974 Uninitialized scalar variable
jackctj117 Sep 9, 2025
50cfecc
Fixes for unused values
jackctj117 Sep 9, 2025
d1cd85e
Support sha2-224, 384 and 512 (#144)
miyazakh Sep 11, 2025
e6967a2
Fix for 572974 Uninitialized scalar variable
jackctj117 Sep 9, 2025
485e144
Fixes for unused values
jackctj117 Sep 9, 2025
042f375
nvm flash fix to break out of loop and counter fix for overflow constant
jackctj117 Sep 11, 2025
58c0985
Merge branch 'main' into coverity-fixes
jackctj117 Sep 11, 2025
424cb07
Fix for rebase mistake
jackctj117 Sep 11, 2025
634b987
Fix for out-of-bounds access 572970
jackctj117 Sep 12, 2025
69d9703
Fix for 584924 Unchecked return value
jackctj117 Sep 16, 2025
7012d26
Fix for 572988 Untrusted value as argument
jackctj117 Sep 16, 2025
aa4b34c
Fix for 572971 Untrusted value as argument
jackctj117 Sep 16, 2025
ec71901
Fix for 572959 Dereference before null check
jackctj117 Sep 16, 2025
3b475c8
Fix for 572992 Unused value
jackctj117 Sep 16, 2025
1c697a3
Fix for 572962 Unused value
jackctj117 Sep 16, 2025
bc246f5
Fix for 572946 Overflowed constant
jackctj117 Sep 17, 2025
1a2bae5
cov-int removal
jackctj117 Sep 18, 2025
5e54474
Fix wolfHSM error checks, remove unrelated code
jackctj117 Sep 18, 2025
67ea3fa
Remove verbose AI comments, improve code clarity
jackctj117 Sep 18, 2025
0761f98
Add integer underflow checks and improve type safety
jackctj117 Sep 19, 2025
0f32747
Fix for 584924 Unchecked return value
jackctj117 Sep 22, 2025
0716db2
Fix for 572980 Uninitialized scalar variable
jackctj117 Sep 22, 2025
3504fc4
Removed 572980 fix, added clang-formatting
jackctj117 Sep 23, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/wh_client_crypto.c
Original file line number Diff line number Diff line change
Expand Up @@ -1901,7 +1901,8 @@ int wh_Client_RsaFunction(whClientContext* ctx, RsaKey* key, int rsa_type,
if ((in != NULL) && (in_len > 0)) {
memcpy(req_in, in, in_len);
}
req->outLen = *inout_out_len;
/* Set output length only when provided to avoid NULL dereference */
req->outLen = (inout_out_len != NULL) ? *inout_out_len : 0;

/* Send Request */
ret = wh_Client_SendRequest(ctx, group, action, req_len,
Expand Down
13 changes: 7 additions & 6 deletions src/wh_client_nvm.c
Original file line number Diff line number Diff line change
Expand Up @@ -626,7 +626,7 @@ int wh_Client_NvmReadRequest(whClientContext* c,
int wh_Client_NvmReadResponse(whClientContext* c, int32_t *out_rc,
whNvmSize *out_len, uint8_t* data)
{
uint8_t buffer[WH_MESSAGE_NVM_MAX_READ_LEN] = {0};
uint8_t buffer[WOLFHSM_CFG_COMM_DATA_LEN] = {0};
whMessageNvm_ReadResponse* msg = (whMessageNvm_ReadResponse*)buffer;
uint16_t hdr_len = sizeof(*msg);
uint8_t* payload = buffer + hdr_len;
Expand All @@ -645,13 +645,14 @@ int wh_Client_NvmReadResponse(whClientContext* c, int32_t *out_rc,
&resp_size, buffer);
if (rc == 0) {
/* Validate response */
if ( (resp_group != WH_MESSAGE_GROUP_NVM) ||
(resp_action != WH_MESSAGE_NVM_ACTION_READ) ||
(resp_size < hdr_len) ||
(resp_size - hdr_len > WH_MESSAGE_NVM_MAX_READ_LEN) ){
if ((resp_group != WH_MESSAGE_GROUP_NVM) ||
(resp_action != WH_MESSAGE_NVM_ACTION_READ) ||
(resp_size < hdr_len) || (resp_size > sizeof(buffer)) ||
(resp_size - hdr_len > WH_MESSAGE_NVM_MAX_READ_LEN)) {
/* Invalid message */
rc = WH_ERROR_ABORTED;
} else {
}
else {
/* Valid message */
if (out_rc != NULL) {
*out_rc = msg->rc;
Expand Down
6 changes: 5 additions & 1 deletion src/wh_nvm_flash.c
Original file line number Diff line number Diff line change
Expand Up @@ -1190,9 +1190,13 @@ int wh_NvmFlash_DestroyObjects(void* c, whNvmId list_count,
/* Write each used object to new partition */
for (entry = 0; entry < WOLFHSM_CFG_NVM_OBJECT_COUNT; entry++) {
if (d->objects[entry].state.status == NF_STATUS_USED) {
/* TODO: Handle errors here better. Break out of loop? */
ret = nfObject_Copy(context, entry,
dest_part, &dest_object, &dest_data);
if (ret != WH_ERROR_OK) {
/* Abort reclaim to avoid activating a partially copied
* partition */
return ret;
}
}
}

Expand Down
44 changes: 24 additions & 20 deletions src/wh_server_counter.c
Original file line number Diff line number Diff line change
Expand Up @@ -56,16 +56,17 @@ int wh_Server_HandleCounter(whServerContext* server, uint16_t magic,

switch (action) {
case WH_COUNTER_INIT: {
whMessageCounter_InitRequest req;
whMessageCounter_InitResponse resp;
whMessageCounter_InitRequest req = {0};
whMessageCounter_InitResponse resp = {0};

/* translate request */
(void)wh_MessageCounter_TranslateInitRequest(
magic, (whMessageCounter_InitRequest*)req_packet, &req);

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

case WH_COUNTER_INCREMENT: {
whMessageCounter_IncrementRequest req;
whMessageCounter_IncrementResponse resp;
whMessageCounter_IncrementRequest req = {0};
whMessageCounter_IncrementResponse resp = {0};

/* translate request */
(void)wh_MessageCounter_TranslateIncrementRequest(
magic, (whMessageCounter_IncrementRequest*)req_packet, &req);

/* read the counter, stored in the metadata label */
ret = wh_Nvm_GetMetadata(server->nvm,
WH_MAKE_KEYID(WH_KEYTYPE_COUNTER,
server->comm->client_id,
req.counterId),
meta);
ret = wh_Nvm_GetMetadata(
server->nvm,
WH_MAKE_KEYID(WH_KEYTYPE_COUNTER,
(uint16_t)server->comm->client_id,
(uint16_t)req.counterId),
meta);
resp.rc = ret;

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

case WH_COUNTER_READ: {
whMessageCounter_ReadRequest req;
whMessageCounter_ReadResponse resp;
whMessageCounter_ReadRequest req = {0};
whMessageCounter_ReadResponse resp = {0};

/* translate request */
(void)wh_MessageCounter_TranslateReadRequest(
magic, (whMessageCounter_ReadRequest*)req_packet, &req);

/* read the counter, stored in the metadata label */
ret = wh_Nvm_GetMetadata(server->nvm,
WH_MAKE_KEYID(WH_KEYTYPE_COUNTER,
server->comm->client_id,
req.counterId),
meta);
ret = wh_Nvm_GetMetadata(
server->nvm,
WH_MAKE_KEYID(WH_KEYTYPE_COUNTER,
(uint16_t)server->comm->client_id,
(uint16_t)req.counterId),
meta);
resp.rc = ret;

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

case WH_COUNTER_DESTROY: {
whMessageCounter_DestroyRequest req;
whMessageCounter_DestroyResponse resp;
whMessageCounter_DestroyRequest req = {0};
whMessageCounter_DestroyResponse resp = {0};

/* translate request */
(void)wh_MessageCounter_TranslateDestroyRequest(
magic, (whMessageCounter_DestroyRequest*)req_packet, &req);

counterId = WH_MAKE_KEYID(WH_KEYTYPE_COUNTER,
server->comm->client_id, req.counterId);
(uint16_t)server->comm->client_id,
(uint16_t)req.counterId);

ret = wh_Nvm_DestroyObjects(server->nvm, 1, &counterId);
resp.rc = ret;
Expand Down
101 changes: 84 additions & 17 deletions src/wh_server_crypto.c
Original file line number Diff line number Diff line change
Expand Up @@ -289,16 +289,25 @@ static int _HandleRsaKeyGen(whServerContext* ctx, uint16_t magic,
printf("[server] RsaKeyGen UniqueId: keyId:%u, ret:%d\n",
key_id, ret);
#endif
if (ret != WH_ERROR_OK) {
/* Early return on unique ID generation failure */
wc_FreeRsaKey(rsa);
return ret;
}
}

ret = wh_Server_CacheImportRsaKey(ctx, rsa, key_id, flags,
label_size, label);
if (ret == 0) {
ret = wh_Server_CacheImportRsaKey(ctx, rsa, key_id, flags,
label_size, label);
}
#ifdef DEBUG_CRYPTOCB_VERBOSE
printf("[server] RsaKeyGen CacheKeyRsa: keyId:%u, ret:%d\n",
key_id, ret);
#endif
res.keyId = WH_KEYID_ID(key_id);
res.len = 0;
if (ret == 0) {
res.keyId = WH_KEYID_ID(key_id);
res.len = 0;
}
}
}
wc_FreeRsaKey(rsa);
Expand Down Expand Up @@ -729,9 +738,16 @@ static int _HandleEccKeyGen(whServerContext* ctx, uint16_t magic,
printf("[server] %s UniqueId: keyId:%u, ret:%d\n", __func__,
key_id, ret);
#endif
if (ret != WH_ERROR_OK) {
/* Early return on unique ID generation failure */
wc_ecc_free(key);
return ret;
}
}
if (ret == 0) {
ret = wh_Server_EccKeyCacheImport(ctx, key, key_id, flags,
label_size, label);
}
ret = wh_Server_EccKeyCacheImport(ctx, key, key_id, flags,
label_size, label);
#ifdef DEBUG_CRYPTOCB
printf("[server] %s CacheImport: keyId:%u, ret:%d\n", __func__,
key_id, ret);
Expand Down Expand Up @@ -1146,10 +1162,17 @@ static int _HandleCurve25519KeyGen(whServerContext* ctx, uint16_t magic,
printf("[server] %s UniqueId: keyId:%u, ret:%d\n", __func__,
key_id, ret);
#endif
if (ret != WH_ERROR_OK) {
/* Early return on unique ID generation failure */
wc_curve25519_free(key);
return ret;
}
}

ret = wh_Server_CacheImportCurve25519Key(
ctx, key, key_id, flags, label_size, label);
if (ret == 0) {
ret = wh_Server_CacheImportCurve25519Key(
ctx, key, key_id, flags, label_size, label);
}
#ifdef DEBUG_CRYPTOCB
printf("[server] %s CacheImport: keyId:%u, ret:%d\n", __func__,
key_id, ret);
Expand Down Expand Up @@ -1693,12 +1716,16 @@ static int _HandleCmac(whServerContext* ctx, uint16_t magic, uint16_t seq,
if (moveToBigCache == 1) {
ret = wh_Server_KeystoreEvictKey(ctx, keyId);
}
meta->id = keyId;
meta->len = sizeof(ctx->crypto->algoCtx.cmac);
ret = wh_Server_KeystoreCacheKey(
ctx, meta, (uint8_t*)ctx->crypto->algoCtx.cmac);
res.keyId = WH_KEYID_ID(keyId);
res.outSz = 0;
if (ret == 0) {
meta->id = keyId;
meta->len = sizeof(ctx->crypto->algoCtx.cmac);
ret = wh_Server_KeystoreCacheKey(
ctx, meta, (uint8_t*)ctx->crypto->algoCtx.cmac);
if (ret == 0) {
res.keyId = WH_KEYID_ID(keyId);
res.outSz = 0;
}
}
#ifdef DEBUG_CRYPTOCB_VERBOSE
printf("[server] cmac saved state in keyid:%x %x len:%u ret:%d type:%d\n",
keyId, WH_KEYID_ID(keyId), meta->len, ret, ctx->crypto->algoCtx.cmac->type);
Expand Down Expand Up @@ -1734,7 +1761,8 @@ static int _HandleSha256(whServerContext* ctx, uint16_t magic,
int ret = 0;
wc_Sha256 sha256[1];
whMessageCrypto_Sha256Request req;
whMessageCrypto_Sha2Response res;
whMessageCrypto_Sha2Response res = {0};

/* Translate the request */
ret = wh_MessageCrypto_TranslateSha256Request(magic, cryptoDataIn, &req);
if (ret != 0) {
Expand All @@ -1751,6 +1779,10 @@ static int _HandleSha256(whServerContext* ctx, uint16_t magic,
sha256->hiLen = req.resumeState.hiLen;

if (req.isLastBlock) {
/* Validate lastBlockLen to prevent potential buffer overread */
if ((unsigned int)req.lastBlockLen > WC_SHA256_BLOCK_SIZE) {
return WH_ERROR_BADARGS;
}
/* wolfCrypt (or cryptoCb) is responsible for last block padding */
if (ret == 0) {
ret = wc_Sha256Update(sha256, req.inBlock, req.lastBlockLen);
Expand Down Expand Up @@ -2113,9 +2145,17 @@ static int _HandleMlDsaKeyGen(whServerContext* ctx, uint16_t magic,
printf("[server] %s UniqueId: keyId:%u, ret:%d\n",
__func__, key_id, ret);
#endif
if (ret != WH_ERROR_OK) {
/* Early return on unique ID generation failure
*/
wc_MlDsaKey_Free(key);
return ret;
}
}
if (ret == 0) {
ret = wh_Server_MlDsaKeyCacheImport(
ctx, key, key_id, flags, label_size, label);
}
ret = wh_Server_MlDsaKeyCacheImport(
ctx, key, key_id, flags, label_size, label);
#ifdef DEBUG_CRYPTOCB
printf("[server] %s CacheImport: keyId:%u, ret:%d\n",
__func__, key_id, ret);
Expand Down Expand Up @@ -2175,6 +2215,16 @@ static int _HandleMlDsaSign(whServerContext* ctx, uint16_t magic,
uint32_t options = req.options;
int evict = !!(options & WH_MESSAGE_CRYPTO_MLDSA_SIGN_OPTIONS_EVICT);

/* Validate input length against available data to prevent buffer overread
*/
if (inSize < sizeof(whMessageCrypto_MlDsaSignRequest)) {
return WH_ERROR_BADARGS;
}
word32 available_data = inSize - sizeof(whMessageCrypto_MlDsaSignRequest);
if (in_len > available_data) {
return WH_ERROR_BADARGS;
}

/* Response message */
byte* res_out =
(uint8_t*)(cryptoDataOut) + sizeof(whMessageCrypto_MlDsaSignResponse);
Expand Down Expand Up @@ -2244,6 +2294,17 @@ static int _HandleMlDsaVerify(whServerContext* ctx, uint16_t magic,
uint32_t sig_len = req.sigSz;
byte* req_sig =
(uint8_t*)(cryptoDataIn) + sizeof(whMessageCrypto_MlDsaVerifyRequest);

/* Validate lengths against available payload (overflow-safe) */
if (inSize < sizeof(whMessageCrypto_MlDsaVerifyRequest)) {
return WH_ERROR_BADARGS;
}
uint32_t available = inSize - sizeof(whMessageCrypto_MlDsaVerifyRequest);
if ((sig_len > available) || (hash_len > available) ||
(sig_len > (available - hash_len))) {
return WH_ERROR_BADARGS;
}

byte* req_hash = req_sig + sig_len;
int evict = !!(options & WH_MESSAGE_CRYPTO_MLDSA_VERIFY_OPTIONS_EVICT);

Expand Down Expand Up @@ -3216,6 +3277,12 @@ static int _HandleMlDsaKeyGenDma(whServerContext* ctx, uint16_t magic,
printf("[server] %s UniqueId: keyId:%u, ret:%d\n",
__func__, keyId, ret);
#endif
if (ret != WH_ERROR_OK) {
/* Early return on unique ID generation failure
*/
wc_MlDsaKey_Free(key);
return ret;
}
}

if (ret == 0) {
Expand Down
15 changes: 10 additions & 5 deletions src/wh_server_keystore.c
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ int wh_Server_KeystoreReadKey(whServerContext* server, whKeyId keyId,
}
/* cache key if free slot, will only kick out other commited keys */
if (ret == 0 && out != NULL) {
wh_Server_KeystoreCacheKey(server, meta, out);
(void)wh_Server_KeystoreCacheKey(server, meta, out);
}
#ifdef WOLFHSM_CFG_SHE_EXTENSION
/* use empty key of zeros if we couldn't find the master ecu key */
Expand Down Expand Up @@ -690,7 +690,7 @@ int wh_Server_HandleKeyRequest(whServerContext* server, uint16_t magic,

case WH_KEY_EVICT: {
whMessageKeystore_EvictRequest req;
whMessageKeystore_EvictResponse resp;
whMessageKeystore_EvictResponse resp = {0};

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

(void)wh_MessageKeystore_TranslateEvictResponse(
magic, &resp, (whMessageKeystore_EvictResponse*)resp_packet);
*out_resp_size = sizeof(resp);
if (ret == WH_ERROR_OK) {
resp.ok = 0;

(void)wh_MessageKeystore_TranslateEvictResponse(
magic, &resp,
(whMessageKeystore_EvictResponse*)resp_packet);
*out_resp_size = sizeof(resp);
}
} break;

case WH_KEY_EXPORT: {
Expand Down
Loading