Skip to content

Commit 2e17ece

Browse files
padelsbachpadelsbach
committed
Add ret_size checking in server keystore
1 parent 3e4549e commit 2e17ece

File tree

4 files changed

+581
-22
lines changed

4 files changed

+581
-22
lines changed

‎src/wh_server_keystore.c‎

Lines changed: 74 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1662,8 +1662,6 @@ int wh_Server_HandleKeyRequest(whServerContext* server, uint16_t magic,
16621662
const void* req_packet, uint16_t* out_resp_size,
16631663
void* resp_packet)
16641664
{
1665-
(void)req_size;
1666-
16671665
int ret = WH_ERROR_OK;
16681666
uint8_t* in;
16691667
uint8_t* out;
@@ -1680,10 +1678,21 @@ int wh_Server_HandleKeyRequest(whServerContext* server, uint16_t magic,
16801678
whMessageKeystore_CacheRequest req;
16811679
whMessageKeystore_CacheResponse resp;
16821680

1681+
/* Validate req_size can hold the fixed request struct */
1682+
if (req_size < sizeof(req)) {
1683+
return WH_ERROR_BADARGS;
1684+
}
1685+
16831686
/* translate request */
16841687
(void)wh_MessageKeystore_TranslateCacheRequest(
16851688
magic, (whMessageKeystore_CacheRequest*)req_packet, &req);
16861689

1690+
/* Validate that the variable-length key data fits within the
1691+
* received packet */
1692+
if (req.sz > req_size - sizeof(req)) {
1693+
return WH_ERROR_BADARGS;
1694+
}
1695+
16871696
/* in is after fixed size fields */
16881697
in = (uint8_t*)req_packet + sizeof(req);
16891698

@@ -1731,6 +1740,10 @@ int wh_Server_HandleKeyRequest(whServerContext* server, uint16_t magic,
17311740
whMessageKeystore_CacheDmaRequest req;
17321741
whMessageKeystore_CacheDmaResponse resp;
17331742

1743+
if (req_size < sizeof(req)) {
1744+
return WH_ERROR_BADARGS;
1745+
}
1746+
17341747
/* translate request */
17351748
(void)wh_MessageKeystore_TranslateCacheDmaRequest(
17361749
magic, (whMessageKeystore_CacheDmaRequest*)req_packet, &req);
@@ -1785,6 +1798,10 @@ int wh_Server_HandleKeyRequest(whServerContext* server, uint16_t magic,
17851798
whMessageKeystore_ExportDmaRequest req;
17861799
whMessageKeystore_ExportDmaResponse resp;
17871800

1801+
if (req_size < sizeof(req)) {
1802+
return WH_ERROR_BADARGS;
1803+
}
1804+
17881805
/* translate request */
17891806
(void)wh_MessageKeystore_TranslateExportDmaRequest(
17901807
magic, (whMessageKeystore_ExportDmaRequest*)req_packet, &req);
@@ -1824,6 +1841,10 @@ int wh_Server_HandleKeyRequest(whServerContext* server, uint16_t magic,
18241841
whMessageKeystore_EvictRequest req;
18251842
whMessageKeystore_EvictResponse resp = {0};
18261843

1844+
if (req_size < sizeof(req)) {
1845+
return WH_ERROR_BADARGS;
1846+
}
1847+
18271848
(void)wh_MessageKeystore_TranslateEvictRequest(
18281849
magic, (whMessageKeystore_EvictRequest*)req_packet, &req);
18291850

@@ -1849,6 +1870,10 @@ int wh_Server_HandleKeyRequest(whServerContext* server, uint16_t magic,
18491870
whMessageKeystore_ExportResponse resp;
18501871
uint32_t keySz;
18511872

1873+
if (req_size < sizeof(req)) {
1874+
return WH_ERROR_BADARGS;
1875+
}
1876+
18521877
/* translate request */
18531878
(void)wh_MessageKeystore_TranslateExportRequest(
18541879
magic, (whMessageKeystore_ExportRequest*)req_packet, &req);
@@ -1887,6 +1912,10 @@ int wh_Server_HandleKeyRequest(whServerContext* server, uint16_t magic,
18871912
whMessageKeystore_CommitRequest req;
18881913
whMessageKeystore_CommitResponse resp;
18891914

1915+
if (req_size < sizeof(req)) {
1916+
return WH_ERROR_BADARGS;
1917+
}
1918+
18901919
/* translate request */
18911920
(void)wh_MessageKeystore_TranslateCommitRequest(
18921921
magic, (whMessageKeystore_CommitRequest*)req_packet, &req);
@@ -1913,6 +1942,10 @@ int wh_Server_HandleKeyRequest(whServerContext* server, uint16_t magic,
19131942
whMessageKeystore_EraseRequest req;
19141943
whMessageKeystore_EraseResponse resp;
19151944

1945+
if (req_size < sizeof(req)) {
1946+
return WH_ERROR_BADARGS;
1947+
}
1948+
19161949
/* translate request */
19171950
(void)wh_MessageKeystore_TranslateEraseRequest(
19181951
magic, (whMessageKeystore_EraseRequest*)req_packet, &req);
@@ -1939,6 +1972,10 @@ int wh_Server_HandleKeyRequest(whServerContext* server, uint16_t magic,
19391972
whMessageKeystore_RevokeRequest req;
19401973
whMessageKeystore_RevokeResponse resp;
19411974

1975+
if (req_size < sizeof(req)) {
1976+
return WH_ERROR_BADARGS;
1977+
}
1978+
19421979
(void)wh_MessageKeystore_TranslateRevokeRequest(
19431980
magic, (whMessageKeystore_RevokeRequest*)req_packet, &req);
19441981

@@ -1965,14 +2002,17 @@ int wh_Server_HandleKeyRequest(whServerContext* server, uint16_t magic,
19652002
whMessageKeystore_KeyWrapResponse wrapResp = {0};
19662003
uint8_t* reqData;
19672004
uint8_t* respData;
1968-
uint32_t reqDataSz = WOLFHSM_CFG_COMM_DATA_LEN - sizeof(wrapReq);
19692005
uint32_t respDataSz = WOLFHSM_CFG_COMM_DATA_LEN - sizeof(wrapResp);
2006+
uint32_t reqDataSz;
19702007

1971-
/* Validate the bounds of the request data */
1972-
if (reqDataSz < req_size) {
1973-
return WH_ERROR_BUFFER_SIZE;
2008+
/* Validate req_size can hold the fixed request struct */
2009+
if (req_size < sizeof(wrapReq)) {
2010+
return WH_ERROR_BADARGS;
19742011
}
19752012

2013+
/* Compute actual variable data size from the received packet */
2014+
reqDataSz = req_size - sizeof(wrapReq);
2015+
19762016
/* Translate request */
19772017
(void)wh_MessageKeystore_TranslateKeyWrapRequest(magic, req_packet,
19782018
&wrapReq);
@@ -2011,15 +2051,18 @@ int wh_Server_HandleKeyRequest(whServerContext* server, uint16_t magic,
20112051
whMessageKeystore_KeyUnwrapAndExportResponse unwrapResp = {0};
20122052
uint8_t* reqData;
20132053
uint8_t* respData;
2014-
uint32_t reqDataSz = WOLFHSM_CFG_COMM_DATA_LEN - sizeof(unwrapReq);
20152054
uint32_t respDataSz =
20162055
WOLFHSM_CFG_COMM_DATA_LEN - sizeof(unwrapResp);
2056+
uint32_t reqDataSz;
20172057

2018-
/* Validate the bounds of the request data */
2019-
if (reqDataSz < req_size) {
2020-
return WH_ERROR_BUFFER_SIZE;
2058+
/* Validate req_size can hold the fixed request struct */
2059+
if (req_size < sizeof(unwrapReq)) {
2060+
return WH_ERROR_BADARGS;
20212061
}
20222062

2063+
/* Compute actual variable data size from the received packet */
2064+
reqDataSz = req_size - sizeof(unwrapReq);
2065+
20232066
/* Translate request */
20242067
(void)wh_MessageKeystore_TranslateKeyUnwrapAndExportRequest(
20252068
magic, req_packet, &unwrapReq);
@@ -2059,14 +2102,17 @@ int wh_Server_HandleKeyRequest(whServerContext* server, uint16_t magic,
20592102
whMessageKeystore_KeyUnwrapAndCacheResponse cacheResp = {0};
20602103
uint8_t* reqData;
20612104
uint8_t* respData;
2062-
uint32_t reqDataSz = WOLFHSM_CFG_COMM_DATA_LEN - sizeof(cacheReq);
20632105
uint32_t respDataSz = WOLFHSM_CFG_COMM_DATA_LEN - sizeof(cacheResp);
2106+
uint32_t reqDataSz;
20642107

2065-
/* Validate the bounds of the request data */
2066-
if (reqDataSz < req_size) {
2067-
return WH_ERROR_BUFFER_SIZE;
2108+
/* Validate req_size can hold the fixed request struct */
2109+
if (req_size < sizeof(cacheReq)) {
2110+
return WH_ERROR_BADARGS;
20682111
}
20692112

2113+
/* Compute actual variable data size from the received packet */
2114+
reqDataSz = req_size - sizeof(cacheReq);
2115+
20702116
/* Translate request */
20712117
(void)wh_MessageKeystore_TranslateKeyUnwrapAndCacheRequest(
20722118
magic, req_packet, &cacheReq);
@@ -2101,14 +2147,17 @@ int wh_Server_HandleKeyRequest(whServerContext* server, uint16_t magic,
21012147
whMessageKeystore_DataWrapResponse wrapResp = {0};
21022148
uint8_t* reqData;
21032149
uint8_t* respData;
2104-
uint32_t reqDataSz = WOLFHSM_CFG_COMM_DATA_LEN - sizeof(wrapReq);
21052150
uint32_t respDataSz = WOLFHSM_CFG_COMM_DATA_LEN - sizeof(wrapResp);
2151+
uint32_t reqDataSz;
21062152

2107-
/* Validate the bounds of the request data */
2108-
if (reqDataSz < req_size) {
2109-
return WH_ERROR_BUFFER_SIZE;
2153+
/* Validate req_size can hold the fixed request struct */
2154+
if (req_size < sizeof(wrapReq)) {
2155+
return WH_ERROR_BADARGS;
21102156
}
21112157

2158+
/* Compute actual variable data size from the received packet */
2159+
reqDataSz = req_size - sizeof(wrapReq);
2160+
21122161
/* Translate request */
21132162
(void)wh_MessageKeystore_TranslateDataWrapRequest(magic, req_packet,
21142163
&wrapReq);
@@ -2147,15 +2196,18 @@ int wh_Server_HandleKeyRequest(whServerContext* server, uint16_t magic,
21472196
whMessageKeystore_DataUnwrapResponse unwrapResp = {0};
21482197
uint8_t* reqData;
21492198
uint8_t* respData;
2150-
uint32_t reqDataSz = WOLFHSM_CFG_COMM_DATA_LEN - sizeof(unwrapReq);
21512199
uint32_t respDataSz =
21522200
WOLFHSM_CFG_COMM_DATA_LEN - sizeof(unwrapResp);
2201+
uint32_t reqDataSz;
21532202

2154-
/* Validate the bounds of the request data */
2155-
if (reqDataSz < req_size) {
2156-
return WH_ERROR_BUFFER_SIZE;
2203+
/* Validate req_size can hold the fixed request struct */
2204+
if (req_size < sizeof(unwrapReq)) {
2205+
return WH_ERROR_BADARGS;
21572206
}
21582207

2208+
/* Compute actual variable data size from the received packet */
2209+
reqDataSz = req_size - sizeof(unwrapReq);
2210+
21592211
/* Translate request */
21602212
(void)wh_MessageKeystore_TranslateDataUnwrapRequest(
21612213
magic, req_packet, &unwrapReq);

‎test/wh_test.c‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
#include "wh_test_crypto_affinity.h"
4646
#include "wh_test_timeout.h"
4747
#include "wh_test_dma.h"
48+
#include "wh_test_keystore_reqsize.h"
4849

4950
#if defined(WOLFHSM_CFG_CERTIFICATE_MANAGER)
5051
#include "wh_test_cert.h"
@@ -91,6 +92,8 @@ int whTest_Unit(void)
9192
#ifdef WOLFHSM_CFG_DMA
9293
WH_TEST_ASSERT(0 == whTest_Dma());
9394
#endif
95+
/* Keystore req_size validation (bug #125) */
96+
WH_TEST_ASSERT(0 == whTest_KeystoreReqSize());
9497

9598
/* Comm tests */
9699
WH_TEST_ASSERT(0 == whTest_Comm());

0 commit comments

Comments
 (0)