Skip to content

Commit 862ff9c

Browse files
yosuke-wolfsslyosuke-wolfssl
committed
Validate SHE client response payload size before use
1 parent 0dcc316 commit 862ff9c

3 files changed

Lines changed: 370 additions & 15 deletions

File tree

‎src/wh_client_she.c‎

Lines changed: 55 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,16 @@
4747

4848
#include "wolfhsm/wh_client_she.h"
4949

50+
/* Reject a response whose payload is too short for its fixed fields. The comm
51+
* layer bounds the payload to the MTU, but not to each message's own size. */
52+
static int _CheckRespSz(int ret, uint16_t dataSz, size_t minSz)
53+
{
54+
if ((ret == WH_ERROR_OK) && ((size_t)dataSz < minSz)) {
55+
ret = WH_ERROR_ABORTED;
56+
}
57+
return ret;
58+
}
59+
5060
int wh_Client_ShePreProgramKey(whClientContext* c, whNvmId keyId,
5161
whNvmFlags flags, uint8_t* key, whNvmSize keySz)
5262
{
@@ -86,7 +96,7 @@ int wh_Client_SheSetUidResponse(whClientContext* c)
8696
{
8797
uint16_t group;
8898
uint16_t action;
89-
uint16_t dataSz;
99+
uint16_t dataSz = 0;
90100
int ret;
91101
whMessageShe_SetUidResponse *resp = NULL;
92102

@@ -96,6 +106,7 @@ int wh_Client_SheSetUidResponse(whClientContext* c)
96106

97107
resp = (whMessageShe_SetUidResponse*)wh_CommClient_GetDataPtr(c->comm);
98108
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)resp);
109+
ret = _CheckRespSz(ret, dataSz, sizeof(*resp));
99110
if (ret == WH_ERROR_OK) {
100111
ret = resp->rc;
101112
}
@@ -121,7 +132,7 @@ int wh_Client_SheSecureBoot(whClientContext* c, uint8_t* bootloader,
121132
uint32_t bootloaderSent = 0;
122133
uint16_t group;
123134
uint16_t action;
124-
uint16_t dataSz;
135+
uint16_t dataSz = 0;
125136
uint8_t* respBuf;
126137

127138
whMessageShe_SecureBootInitRequest* initReq = NULL;
@@ -149,6 +160,7 @@ int wh_Client_SheSecureBoot(whClientContext* c, uint8_t* bootloader,
149160
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, respBuf);
150161
initResp = (whMessageShe_SecureBootInitResponse*)respBuf;
151162
} while (ret == WH_ERROR_NOTREADY);
163+
ret = _CheckRespSz(ret, dataSz, sizeof(*initResp));
152164
}
153165

154166
/* send update sub command until we've sent the entire bootloader */
@@ -185,6 +197,8 @@ int wh_Client_SheSecureBoot(whClientContext* c, uint8_t* bootloader,
185197
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz,
186198
respBuf);
187199
} while (ret == WH_ERROR_NOTREADY);
200+
ret = _CheckRespSz(ret, dataSz,
201+
sizeof(whMessageShe_SecureBootUpdateResponse));
188202
}
189203

190204
/* increment sent */
@@ -204,6 +218,7 @@ int wh_Client_SheSecureBoot(whClientContext* c, uint8_t* bootloader,
204218
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, respBuf);
205219
finishResp = (whMessageShe_SecureBootFinishResponse*)respBuf;
206220
} while (ret == WH_ERROR_NOTREADY);
221+
ret = _CheckRespSz(ret, dataSz, sizeof(*finishResp));
207222
}
208223

209224
if (ret == 0) {
@@ -231,7 +246,7 @@ int wh_Client_SheGetStatusResponse(whClientContext* c, uint8_t* sreg)
231246
{
232247
uint16_t group;
233248
uint16_t action;
234-
uint16_t dataSz;
249+
uint16_t dataSz = 0;
235250
int ret;
236251
whMessageShe_GetStatusResponse *resp = NULL;
237252

@@ -242,6 +257,7 @@ int wh_Client_SheGetStatusResponse(whClientContext* c, uint8_t* sreg)
242257
resp = (whMessageShe_GetStatusResponse*)wh_CommClient_GetDataPtr(c->comm);
243258

244259
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)resp);
260+
ret = _CheckRespSz(ret, dataSz, sizeof(*resp));
245261

246262
/* return error or set sreg */
247263
if (ret == 0) {
@@ -296,7 +312,7 @@ int wh_Client_SheLoadKeyResponse(whClientContext* c, uint8_t* messageFour,
296312
int ret;
297313
uint16_t group;
298314
uint16_t action;
299-
uint16_t dataSz;
315+
uint16_t dataSz = 0;
300316
whMessageShe_LoadKeyResponse* resp = NULL;
301317

302318
if (c == NULL || messageFour == NULL || messageFive == NULL) {
@@ -306,6 +322,7 @@ int wh_Client_SheLoadKeyResponse(whClientContext* c, uint8_t* messageFour,
306322
resp = (whMessageShe_LoadKeyResponse*)wh_CommClient_GetDataPtr(c->comm);
307323

308324
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)resp);
325+
ret = _CheckRespSz(ret, dataSz, sizeof(*resp));
309326
if (ret == 0) {
310327
if (resp->rc != WH_SHE_ERC_NO_ERROR) {
311328
ret = resp->rc;
@@ -355,7 +372,7 @@ int wh_Client_SheLoadPlainKeyResponse(whClientContext* c)
355372
int ret;
356373
uint16_t group;
357374
uint16_t action;
358-
uint16_t dataSz;
375+
uint16_t dataSz = 0;
359376
whMessageShe_LoadPlainKeyResponse* resp = NULL;
360377

361378
if (c == NULL) {
@@ -366,6 +383,7 @@ int wh_Client_SheLoadPlainKeyResponse(whClientContext* c)
366383
(whMessageShe_LoadPlainKeyResponse*)wh_CommClient_GetDataPtr(c->comm);
367384

368385
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)resp);
386+
ret = _CheckRespSz(ret, dataSz, sizeof(*resp));
369387
if (ret == 0) {
370388
ret = resp->rc;
371389
}
@@ -404,7 +422,7 @@ int wh_Client_SheExportRamKeyResponse(whClientContext* c, uint8_t* messageOne,
404422
int ret;
405423
uint16_t group;
406424
uint16_t action;
407-
uint16_t dataSz;
425+
uint16_t dataSz = 0;
408426
whMessageShe_ExportRamKeyResponse* resp = NULL;
409427

410428
if (c == NULL || messageOne == NULL || messageTwo == NULL ||
@@ -416,6 +434,7 @@ int wh_Client_SheExportRamKeyResponse(whClientContext* c, uint8_t* messageOne,
416434
(whMessageShe_ExportRamKeyResponse*)wh_CommClient_GetDataPtr(c->comm);
417435

418436
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)resp);
437+
ret = _CheckRespSz(ret, dataSz, sizeof(*resp));
419438
if (ret == 0) {
420439
if (resp->rc != WH_SHE_ERC_NO_ERROR) {
421440
ret = resp->rc;
@@ -464,7 +483,7 @@ int wh_Client_SheInitRndResponse(whClientContext* c)
464483
int ret;
465484
uint16_t group;
466485
uint16_t action;
467-
uint16_t dataSz;
486+
uint16_t dataSz = 0;
468487
whMessageShe_InitRngResponse* resp = NULL;
469488

470489
if (c == NULL) {
@@ -473,6 +492,7 @@ int wh_Client_SheInitRndResponse(whClientContext* c)
473492

474493
resp = (whMessageShe_InitRngResponse*)wh_CommClient_GetDataPtr(c->comm);
475494
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)resp);
495+
ret = _CheckRespSz(ret, dataSz, sizeof(*resp));
476496
if (ret == 0) {
477497
ret = resp->rc;
478498
}
@@ -506,7 +526,7 @@ int wh_Client_SheRndResponse(whClientContext* c, uint8_t* out, uint32_t* outSz)
506526
int ret;
507527
uint16_t group;
508528
uint16_t action;
509-
uint16_t dataSz;
529+
uint16_t dataSz = 0;
510530
whMessageShe_RndResponse* resp = NULL;
511531

512532
if (c == NULL || out == NULL || outSz == NULL || *outSz < WH_SHE_KEY_SZ) {
@@ -516,6 +536,7 @@ int wh_Client_SheRndResponse(whClientContext* c, uint8_t* out, uint32_t* outSz)
516536
resp = (whMessageShe_RndResponse*)wh_CommClient_GetDataPtr(c->comm);
517537

518538
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)resp);
539+
ret = _CheckRespSz(ret, dataSz, sizeof(*resp));
519540

520541
if (ret == 0) {
521542
if (resp->rc != WH_SHE_ERC_NO_ERROR)
@@ -567,7 +588,7 @@ int wh_Client_SheExtendSeedResponse(whClientContext* c)
567588
int ret;
568589
uint16_t group;
569590
uint16_t action;
570-
uint16_t dataSz;
591+
uint16_t dataSz = 0;
571592
whMessageShe_ExtendSeedResponse* resp = NULL;
572593

573594
if (c == NULL) {
@@ -576,6 +597,7 @@ int wh_Client_SheExtendSeedResponse(whClientContext* c)
576597

577598
resp = (whMessageShe_ExtendSeedResponse*)wh_CommClient_GetDataPtr(c->comm);
578599
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)resp);
600+
ret = _CheckRespSz(ret, dataSz, sizeof(*resp));
579601

580602
if (ret == 0) {
581603
ret = resp->rc;
@@ -625,7 +647,7 @@ int wh_Client_SheEncEcbResponse(whClientContext* c, uint8_t* out, uint32_t sz)
625647
int ret;
626648
uint16_t group;
627649
uint16_t action;
628-
uint16_t dataSz;
650+
uint16_t dataSz = 0;
629651
uint8_t* packOut;
630652
whMessageShe_EncEcbResponse* resp = NULL;
631653

@@ -639,10 +661,14 @@ int wh_Client_SheEncEcbResponse(whClientContext* c, uint8_t* out, uint32_t sz)
639661
packOut = (uint8_t*)(resp + 1);
640662

641663
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)resp);
664+
ret = _CheckRespSz(ret, dataSz, sizeof(*resp));
642665
if (ret == 0) {
643666
if (resp->rc != WH_SHE_ERC_NO_ERROR) {
644667
ret = resp->rc;
645668
}
669+
else if ((uint64_t)sizeof(*resp) + resp->sz > (uint64_t)dataSz) {
670+
ret = WH_ERROR_ABORTED;
671+
}
646672
else if (sz < resp->sz) {
647673
ret = WH_ERROR_BADARGS;
648674
}
@@ -698,7 +724,7 @@ int wh_Client_SheEncCbcResponse(whClientContext* c, uint8_t* out, uint32_t sz)
698724
int ret;
699725
uint16_t group;
700726
uint16_t action;
701-
uint16_t dataSz;
727+
uint16_t dataSz = 0;
702728
uint8_t* packOut;
703729
whMessageShe_EncCbcResponse* resp = NULL;
704730

@@ -711,10 +737,14 @@ int wh_Client_SheEncCbcResponse(whClientContext* c, uint8_t* out, uint32_t sz)
711737
packOut = (uint8_t*)(resp + 1);
712738

713739
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)resp);
740+
ret = _CheckRespSz(ret, dataSz, sizeof(*resp));
714741
if (ret == 0) {
715742
if (resp->rc != WH_SHE_ERC_NO_ERROR) {
716743
ret = resp->rc;
717744
}
745+
else if ((uint64_t)sizeof(*resp) + resp->sz > (uint64_t)dataSz) {
746+
ret = WH_ERROR_ABORTED;
747+
}
718748
else if (sz < resp->sz) {
719749
ret = WH_ERROR_BADARGS;
720750
}
@@ -766,7 +796,7 @@ int wh_Client_SheDecEcbResponse(whClientContext* c, uint8_t* out, uint32_t sz)
766796
int ret;
767797
uint16_t group;
768798
uint16_t action;
769-
uint16_t dataSz;
799+
uint16_t dataSz = 0;
770800
uint8_t* packOut;
771801
whMessageShe_DecEcbResponse* resp = NULL;
772802

@@ -779,10 +809,14 @@ int wh_Client_SheDecEcbResponse(whClientContext* c, uint8_t* out, uint32_t sz)
779809
packOut = (uint8_t*)(resp + 1);
780810

781811
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)resp);
812+
ret = _CheckRespSz(ret, dataSz, sizeof(*resp));
782813
if (ret == 0) {
783814
if (resp->rc != WH_SHE_ERC_NO_ERROR) {
784815
ret = resp->rc;
785816
}
817+
else if ((uint64_t)sizeof(*resp) + resp->sz > (uint64_t)dataSz) {
818+
ret = WH_ERROR_ABORTED;
819+
}
786820
else if (sz < resp->sz) {
787821
ret = WH_ERROR_BADARGS;
788822
}
@@ -838,7 +872,7 @@ int wh_Client_SheDecCbcResponse(whClientContext* c, uint8_t* out, uint32_t sz)
838872
int ret;
839873
uint16_t group;
840874
uint16_t action;
841-
uint16_t dataSz;
875+
uint16_t dataSz = 0;
842876
uint8_t* packOut;
843877
whMessageShe_DecCbcResponse* resp = NULL;
844878

@@ -851,10 +885,14 @@ int wh_Client_SheDecCbcResponse(whClientContext* c, uint8_t* out, uint32_t sz)
851885
packOut = (uint8_t*)(resp + 1);
852886

853887
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)resp);
888+
ret = _CheckRespSz(ret, dataSz, sizeof(*resp));
854889
if (ret == 0) {
855890
if (resp->rc != WH_SHE_ERC_NO_ERROR) {
856891
ret = resp->rc;
857892
}
893+
else if ((uint64_t)sizeof(*resp) + resp->sz > (uint64_t)dataSz) {
894+
ret = WH_ERROR_ABORTED;
895+
}
858896
else if (sz < resp->sz) {
859897
ret = WH_ERROR_BADARGS;
860898
}
@@ -908,7 +946,7 @@ int wh_Client_SheGenerateMacResponse(whClientContext* c, uint8_t* out,
908946
int ret;
909947
uint16_t group;
910948
uint16_t action;
911-
uint16_t dataSz;
949+
uint16_t dataSz = 0;
912950
whMessageShe_GenMacResponse* resp = NULL;
913951

914952
if (c == NULL || out == NULL || sz < WH_SHE_KEY_SZ) {
@@ -918,6 +956,7 @@ int wh_Client_SheGenerateMacResponse(whClientContext* c, uint8_t* out,
918956
resp = (whMessageShe_GenMacResponse*)wh_CommClient_GetDataPtr(c->comm);
919957

920958
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)resp);
959+
ret = _CheckRespSz(ret, dataSz, sizeof(*resp));
921960
if (ret == 0) {
922961
if (resp->rc != WH_SHE_ERC_NO_ERROR) {
923962
ret = resp->rc;
@@ -981,7 +1020,7 @@ int wh_Client_SheVerifyMacResponse(whClientContext* c, uint8_t* outStatus)
9811020
int ret;
9821021
uint16_t group;
9831022
uint16_t action;
984-
uint16_t dataSz;
1023+
uint16_t dataSz = 0;
9851024
whMessageShe_VerifyMacResponse* resp = NULL;
9861025

9871026
if (c == NULL || outStatus == NULL) {
@@ -990,6 +1029,7 @@ int wh_Client_SheVerifyMacResponse(whClientContext* c, uint8_t* outStatus)
9901029

9911030
resp = (whMessageShe_VerifyMacResponse*)wh_CommClient_GetDataPtr(c->comm);
9921031
ret = wh_Client_RecvResponse(c, &group, &action, &dataSz, (uint8_t*)resp);
1032+
ret = _CheckRespSz(ret, dataSz, sizeof(*resp));
9931033
if (ret == 0) {
9941034
if (resp->rc != WH_SHE_ERC_NO_ERROR) {
9951035
ret = resp->rc;

0 commit comments

Comments
 (0)