Skip to content

Commit 84f86a3

Browse files
bigbrettbigbrett
authored
Merge pull request #473 from yosuke-wolfssl/fix/resSize
Validate response payload sizes in wh_client.c handlers
2 parents dc3f4f5 + 3dba187 commit 84f86a3

1 file changed

Lines changed: 80 additions & 13 deletions

File tree

‎src/wh_client.c‎

Lines changed: 80 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -584,7 +584,8 @@ int wh_Client_CommCloseResponse(whClientContext* c)
584584
if (rc == 0) {
585585
/* Validate response */
586586
if ( (resp_group != WH_MESSAGE_GROUP_COMM) ||
587-
(resp_action != WH_MESSAGE_COMM_ACTION_CLOSE) ){
587+
(resp_action != WH_MESSAGE_COMM_ACTION_CLOSE) ||
588+
(resp_size != 0) ){
588589
/* Invalid message */
589590
rc = WH_ERROR_ABORTED;
590591
} else {
@@ -867,7 +868,12 @@ int wh_Client_KeyCacheResponse(whClientContext* c, uint16_t* keyId)
867868
ret = wh_Client_RecvResponse(c, &group, &action, &size,
868869
WOLFHSM_CFG_COMM_DATA_LEN, (uint8_t*)resp);
869870
if (ret == WH_ERROR_OK) {
870-
if (resp->rc != 0) {
871+
/* Defensive bound: the response fields must fit within the actual
872+
* received frame */
873+
if (size < sizeof(*resp)) {
874+
ret = WH_ERROR_ABORTED;
875+
}
876+
else if (resp->rc != 0) {
871877
ret = resp->rc;
872878
}
873879
else {
@@ -955,7 +961,12 @@ int wh_Client_KeyCacheRandomResponse(whClientContext* c, uint16_t* outKeyId)
955961
ret = wh_Client_RecvResponse(c, &group, &action, &size,
956962
WOLFHSM_CFG_COMM_DATA_LEN, (uint8_t*)resp);
957963
if (ret == WH_ERROR_OK) {
958-
if (resp->rc != 0) {
964+
/* Defensive bound: the response fields must fit within the actual
965+
* received frame */
966+
if (size < sizeof(*resp)) {
967+
ret = WH_ERROR_ABORTED;
968+
}
969+
else if (resp->rc != 0) {
959970
ret = resp->rc;
960971
}
961972
else {
@@ -1025,7 +1036,12 @@ int wh_Client_KeyEvictResponse(whClientContext* c)
10251036
(uint8_t*)&resp);
10261037

10271038
if (ret == 0) {
1028-
if (resp.rc != 0) {
1039+
/* Defensive bound: the response fields must fit within the actual
1040+
* received frame */
1041+
if (size < sizeof(resp)) {
1042+
ret = WH_ERROR_ABORTED;
1043+
}
1044+
else if (resp.rc != 0) {
10291045
ret = resp.rc;
10301046
}
10311047
}
@@ -1088,9 +1104,17 @@ int wh_Client_KeyExportResponse(whClientContext* c, uint8_t* label,
10881104
ret = wh_Client_RecvResponse(c, &group, &action, &size,
10891105
WOLFHSM_CFG_COMM_DATA_LEN, (uint8_t*)resp);
10901106
if (ret == WH_ERROR_OK) {
1091-
if (resp->rc != 0) {
1107+
/* Defensive bounds: the fixed fields, then the key material, must fit
1108+
* within the actual received frame */
1109+
if (size < sizeof(*resp)) {
1110+
ret = WH_ERROR_ABORTED;
1111+
}
1112+
else if (resp->rc != 0) {
10921113
ret = resp->rc;
10931114
}
1115+
else if (resp->len > (size - sizeof(*resp))) {
1116+
ret = WH_ERROR_ABORTED;
1117+
}
10941118
else {
10951119
if (out == NULL) {
10961120
*outSz = resp->len;
@@ -1174,9 +1198,17 @@ int wh_Client_KeyExportPublicResponse(whClientContext* c, uint8_t* label,
11741198
ret = wh_Client_RecvResponse(c, &group, &action, &size,
11751199
WOLFHSM_CFG_COMM_DATA_LEN, (uint8_t*)resp);
11761200
if (ret == WH_ERROR_OK) {
1177-
if (resp->rc != 0) {
1201+
/* Defensive bounds: the fixed fields, then the key material, must fit
1202+
* within the actual received frame */
1203+
if (size < sizeof(*resp)) {
1204+
ret = WH_ERROR_ABORTED;
1205+
}
1206+
else if (resp->rc != 0) {
11781207
ret = resp->rc;
11791208
}
1209+
else if (resp->len > (size - sizeof(*resp))) {
1210+
ret = WH_ERROR_ABORTED;
1211+
}
11801212
else {
11811213
if (out == NULL) {
11821214
*outSz = resp->len;
@@ -1252,7 +1284,12 @@ int wh_Client_KeyCommitResponse(whClientContext* c)
12521284
ret = wh_Client_RecvResponse(c, &group, &action, &size,
12531285
WOLFHSM_CFG_COMM_DATA_LEN, (uint8_t*)resp);
12541286
if (ret == WH_ERROR_OK) {
1255-
if (resp->rc != 0) {
1287+
/* Defensive bound: the response fields must fit within the actual
1288+
* received frame */
1289+
if (size < sizeof(*resp)) {
1290+
ret = WH_ERROR_ABORTED;
1291+
}
1292+
else if (resp->rc != 0) {
12561293
ret = resp->rc;
12571294
}
12581295
}
@@ -1309,7 +1346,12 @@ int wh_Client_KeyEraseResponse(whClientContext* c)
13091346
ret = wh_Client_RecvResponse(c, &group, &action, &size,
13101347
WOLFHSM_CFG_COMM_DATA_LEN, (uint8_t*)resp);
13111348
if (ret == 0) {
1312-
if (resp->rc != 0) {
1349+
/* Defensive bound: the response fields must fit within the actual
1350+
* received frame */
1351+
if (size < sizeof(*resp)) {
1352+
ret = WH_ERROR_ABORTED;
1353+
}
1354+
else if (resp->rc != 0) {
13131355
ret = resp->rc;
13141356
}
13151357
}
@@ -1366,7 +1408,12 @@ int wh_Client_KeyRevokeResponse(whClientContext* c)
13661408
ret = wh_Client_RecvResponse(c, &group, &action, &size,
13671409
WOLFHSM_CFG_COMM_DATA_LEN, (uint8_t*)resp);
13681410
if (ret == 0) {
1369-
if (resp->rc != 0) {
1411+
/* Defensive bound: the response fields must fit within the actual
1412+
* received frame */
1413+
if (size < sizeof(*resp)) {
1414+
ret = WH_ERROR_ABORTED;
1415+
}
1416+
else if (resp->rc != 0) {
13701417
ret = resp->rc;
13711418
}
13721419
}
@@ -1425,7 +1472,12 @@ int wh_Client_CounterInitResponse(whClientContext* c, uint32_t* counter)
14251472
ret = wh_Client_RecvResponse(c, &group, &action, &size,
14261473
WOLFHSM_CFG_COMM_DATA_LEN, (uint8_t*)resp);
14271474
if (ret == WH_ERROR_OK) {
1428-
if (resp->rc != 0) {
1475+
/* Defensive bound: the response fields must fit within the actual
1476+
* received frame */
1477+
if (size < sizeof(*resp)) {
1478+
ret = WH_ERROR_ABORTED;
1479+
}
1480+
else if (resp->rc != 0) {
14291481
ret = resp->rc;
14301482
}
14311483
else if (counter != NULL) {
@@ -1504,7 +1556,12 @@ int wh_Client_CounterIncrementResponse(whClientContext* c, uint32_t* counter)
15041556
ret = wh_Client_RecvResponse(c, &group, &action, &size,
15051557
WOLFHSM_CFG_COMM_DATA_LEN, (uint8_t*)resp);
15061558
if (ret == WH_ERROR_OK) {
1507-
if (resp->rc != 0) {
1559+
/* Defensive bound: the response fields must fit within the actual
1560+
* received frame */
1561+
if (size < sizeof(*resp)) {
1562+
ret = WH_ERROR_ABORTED;
1563+
}
1564+
else if (resp->rc != 0) {
15081565
ret = resp->rc;
15091566
}
15101567
else if (counter != NULL) {
@@ -1565,7 +1622,12 @@ int wh_Client_CounterReadResponse(whClientContext* c, uint32_t* counter)
15651622
ret = wh_Client_RecvResponse(c, &group, &action, &size,
15661623
WOLFHSM_CFG_COMM_DATA_LEN, (uint8_t*)resp);
15671624
if (ret == WH_ERROR_OK) {
1568-
if (resp->rc != 0) {
1625+
/* Defensive bound: the response fields must fit within the actual
1626+
* received frame */
1627+
if (size < sizeof(*resp)) {
1628+
ret = WH_ERROR_ABORTED;
1629+
}
1630+
else if (resp->rc != 0) {
15691631
ret = resp->rc;
15701632
}
15711633
else {
@@ -1626,7 +1688,12 @@ int wh_Client_CounterDestroyResponse(whClientContext* c)
16261688
ret = wh_Client_RecvResponse(c, &group, &action, &size,
16271689
WOLFHSM_CFG_COMM_DATA_LEN, (uint8_t*)resp);
16281690
if (ret == WH_ERROR_OK) {
1629-
if (resp->rc != 0) {
1691+
/* Defensive bound: the response fields must fit within the actual
1692+
* received frame */
1693+
if (size < sizeof(*resp)) {
1694+
ret = WH_ERROR_ABORTED;
1695+
}
1696+
else if (resp->rc != 0) {
16301697
ret = resp->rc;
16311698
}
16321699
}

0 commit comments

Comments
 (0)