From 24160497535562ca2b5dee5049e57fc36013cd80 Mon Sep 17 00:00:00 2001 From: HIDEKI MIYAZAKI Date: Thu, 21 May 2026 09:23:20 -0700 Subject: [PATCH 1/3] add bound check for aes-ctr --- src/wh_server_crypto.c | 91 +++++++++++++++++++++++++----------------- test/wh_test_crypto.c | 47 ++++++++++++++++++++++ 2 files changed, 102 insertions(+), 36 deletions(-) diff --git a/src/wh_server_crypto.c b/src/wh_server_crypto.c index 788f91f53..aa7d7109f 100644 --- a/src/wh_server_crypto.c +++ b/src/wh_server_crypto.c @@ -2534,22 +2534,33 @@ static int _HandleAesCtr(whServerContext* ctx, uint16_t magic, int devId, ret = wc_AesSetKeyDirect(aes, (byte*)key, (word32)key_len, (byte*)iv, enc != 0 ? AES_ENCRYPTION : AES_DECRYPTION); if (ret == WH_ERROR_OK) { - /* do the crypto operation; also restore previous left */ - aes->left = left; - memcpy(aes->tmp, tmp, sizeof(aes->tmp)); - if (enc != 0) { - ret = wc_AesCtrEncrypt(aes, (byte*)out, (byte*)in, (word32)len); - if (ret == WH_ERROR_OK) { - WH_DEBUG_VERBOSE_HEXDUMP("[AesCtr] Encrypted output", - out, len); - } + /* Reject client-supplied left values outside the valid range. + * wc_AesCtrEncrypt indexes aes->tmp via AES_BLOCK_SIZE - aes->left; + * an out-of-range value causes an out-of-bounds read that could + * disclose server-side memory across the HSM trust boundary. */ + if (left > AES_BLOCK_SIZE) { + ret = WH_ERROR_BADARGS; } else { - /* CTR uses the same function for encrypt and decrypt */ - ret = wc_AesCtrEncrypt(aes, (byte*)out, (byte*)in, (word32)len); - if (ret == WH_ERROR_OK) { - WH_DEBUG_VERBOSE_HEXDUMP("[AesCtr] Decrypted output", - out, len); + /* Restore streaming CTR context from the previous call. */ + aes->left = left; + memcpy(aes->tmp, tmp, sizeof(aes->tmp)); + if (enc != 0) { + ret = wc_AesCtrEncrypt(aes, (byte*)out, (byte*)in, + (word32)len); + if (ret == WH_ERROR_OK) { + WH_DEBUG_VERBOSE_HEXDUMP("[AesCtr] Encrypted output", + out, len); + } + } + else { + /* CTR uses the same function for encrypt and decrypt */ + ret = wc_AesCtrEncrypt(aes, (byte*)out, (byte*)in, + (word32)len); + if (ret == WH_ERROR_OK) { + WH_DEBUG_VERBOSE_HEXDUMP("[AesCtr] Decrypted output", + out, len); + } } } } @@ -2695,33 +2706,41 @@ static int _HandleAesCtrDma(whServerContext* ctx, uint16_t magic, int devId, ret = wc_AesSetKeyDirect(aes, (byte*)key, (word32)keyLen, (byte*)iv, enc != 0 ? AES_ENCRYPTION : AES_DECRYPTION); if (ret == WH_ERROR_OK) { - /* do the crypto operation */ - /* restore previous left */ - aes->left = left; - memcpy(aes->tmp, tmp, sizeof(aes->tmp)); - if (enc != 0) { - ret = wc_AesCtrEncrypt(aes, (byte*)outAddr, (byte*)inAddr, - (word32)len); - if (ret == WH_ERROR_OK) { - WH_DEBUG_VERBOSE_HEXDUMP("[AesCtr] Encrypted output", - outAddr, len); - } + /* Reject client-supplied left values outside the valid range. + * wc_AesCtrEncrypt indexes aes->tmp via AES_BLOCK_SIZE - aes->left; + * an out-of-range value causes an out-of-bounds read that could + * disclose server-side memory across the HSM trust boundary. */ + if (left > AES_BLOCK_SIZE) { + ret = WH_ERROR_BADARGS; } else { - /* CTR uses the same function for encrypt and decrypt */ - ret = wc_AesCtrEncrypt(aes, (byte*)outAddr, (byte*)inAddr, - (word32)len); + /* Restore streaming CTR context from the previous call. */ + aes->left = left; + memcpy(aes->tmp, tmp, sizeof(aes->tmp)); + if (enc != 0) { + ret = wc_AesCtrEncrypt(aes, (byte*)outAddr, (byte*)inAddr, + (word32)len); + if (ret == WH_ERROR_OK) { + WH_DEBUG_VERBOSE_HEXDUMP("[AesCtr] Encrypted output", + outAddr, len); + } + } + else { + /* CTR uses the same function for encrypt and decrypt */ + ret = wc_AesCtrEncrypt(aes, (byte*)outAddr, (byte*)inAddr, + (word32)len); + if (ret == WH_ERROR_OK) { + WH_DEBUG_VERBOSE_HEXDUMP("[AesCtr] Decrypted output", + outAddr, len); + } + } if (ret == WH_ERROR_OK) { - WH_DEBUG_VERBOSE_HEXDUMP("[AesCtr] Decrypted output", - outAddr, len); + left = aes->left; + outSz = len; + memcpy(out_tmp, aes->tmp, AES_BLOCK_SIZE); + memcpy(out_iv, aes->reg, AES_IV_SIZE); } } - if (ret == WH_ERROR_OK) { - left = aes->left; - outSz = len; - memcpy(out_tmp, aes->tmp, AES_BLOCK_SIZE); - memcpy(out_iv, aes->reg, AES_IV_SIZE); - } } } diff --git a/test/wh_test_crypto.c b/test/wh_test_crypto.c index b5fe773b8..f70a7e3b6 100644 --- a/test/wh_test_crypto.c +++ b/test/wh_test_crypto.c @@ -8447,6 +8447,31 @@ static int whTest_CryptoAesAsync(whClientContext* ctx, int devId, WC_RNG* rng) } (void)wc_AesFree(aes); } + + /* CTR: server must reject left > AES_BLOCK_SIZE. + * wc_AesCtrEncrypt indexes aes->tmp via AES_BLOCK_SIZE - aes->left; + * an oversized client-supplied value would cause an out-of-bounds read + * disclosing server-side memory across the HSM trust boundary. */ + if (ret == 0) { + int tmp; + ret = wc_AesInit(aes, NULL, devId); + if (ret == 0) { + ret = wc_AesSetKeyDirect(aes, key, sizeof(key), iv, AES_ENCRYPTION); + } + if (ret == 0) { + aes->left = AES_BLOCK_SIZE + 1; + tmp = wh_Client_AesCtr(ctx, aes, 1, plainIn, sizeof(plainIn), + cipher); + if (tmp != WH_ERROR_BADARGS) { + WH_ERROR_PRINT( + "AES-CTR async: left > AES_BLOCK_SIZE should be " + "BADARGS, got %d\n", + tmp); + ret = -1; + } + } + (void)wc_AesFree(aes); + } if (ret == 0) { WH_TEST_PRINT("AES CTR ASYNC DEVID=0x%X SUCCESS\n", devId); } @@ -9446,6 +9471,28 @@ static int whTest_CryptoAesDmaAsync(whClientContext* ctx, int devId, memset(cipher, 0, sizeof(cipher)); memset(plainOut, 0, sizeof(plainOut)); } + + /* CTR DMA: same left > AES_BLOCK_SIZE rejection as the non-DMA path. */ + if (ret == 0) { + int tmp; + ret = wc_AesInit(aes, NULL, devId); + if (ret == 0) { + ret = wc_AesSetKeyDirect(aes, key, sizeof(key), iv, AES_ENCRYPTION); + } + if (ret == 0) { + aes->left = AES_BLOCK_SIZE + 1; + tmp = wh_Client_AesCtrDma(ctx, aes, 1, plainIn, sizeof(plainIn), + cipher); + if (tmp != WH_ERROR_BADARGS) { + WH_ERROR_PRINT( + "AES-CTR DMA async: left > AES_BLOCK_SIZE should be " + "BADARGS, got %d\n", + tmp); + ret = -1; + } + } + (void)wc_AesFree(aes); + } if (ret == 0) { WH_TEST_PRINT("AES CTR DMA ASYNC DEVID=0x%X SUCCESS\n", devId); } From 5721261bcedc7fdbd46fce74fa7a2685ac73931a Mon Sep 17 00:00:00 2001 From: Hideki Miyazaki Date: Wed, 3 Jun 2026 06:40:11 +0900 Subject: [PATCH 2/3] Add tests to test-refactor --- .../client-server/wh_test_crypto_aes.c | 85 +++++++++++++++++++ test-refactor/wh_test_list.c | 2 + 2 files changed, 87 insertions(+) diff --git a/test-refactor/client-server/wh_test_crypto_aes.c b/test-refactor/client-server/wh_test_crypto_aes.c index 12a1e4d06..76b114eec 100644 --- a/test-refactor/client-server/wh_test_crypto_aes.c +++ b/test-refactor/client-server/wh_test_crypto_aes.c @@ -442,6 +442,91 @@ static int whTest_CryptoAesCbcStreaming(whClientContext* ctx) } #endif /* HAVE_AES_CBC */ +#ifdef WOLFSSL_AES_COUNTER +/* Verifies that wh_Client_AesCtr (and wh_Client_AesCtrDma when DMA is + * enabled) reject aes->left > AES_BLOCK_SIZE with WH_ERROR_BADARGS. + * wc_AesCtrEncrypt indexes aes->tmp via AES_BLOCK_SIZE - aes->left; an + * oversized value would cause an out-of-bounds read disclosing server-side + * memory across the HSM trust boundary. */ +int whTest_CryptoAesCtrLeftOob(whClientContext* ctx) +{ + int devId = WH_DEV_ID; + int ret = 0; + int tmp; + Aes aes[1]; + uint8_t cipher[AES_BLOCK_SIZE] = {0}; + const uint8_t key[] = {0x2b, 0x7e, 0x15, 0x16, + 0x28, 0xae, 0xd2, 0xa6, + 0xab, 0xf7, 0x15, 0x88, + 0x09, 0xcf, 0x4f, 0x3c}; + const uint8_t iv[AES_BLOCK_SIZE] = {0x00, 0x01, 0x02, 0x03, + 0x04, 0x05, 0x06, 0x07, + 0x08, 0x09, 0x0a, 0x0b, + 0x0c, 0x0d, 0x0e, 0x0f}; + const uint8_t plainIn[AES_BLOCK_SIZE] = {0x6b, 0xc1, 0xbe, 0xe2, + 0x2e, 0x40, 0x9f, 0x96, + 0xe9, 0x3d, 0x7e, 0x11, + 0x73, 0x93, 0x17, 0x2a}; + + ret = wc_AesInit(aes, NULL, devId); + if (ret != 0) { + WH_ERROR_PRINT("Failed to wc_AesInit %d\n", ret); + } + else { + ret = wc_AesSetKeyDirect(aes, key, sizeof(key), iv, AES_ENCRYPTION); + if (ret != 0) { + WH_ERROR_PRINT("Failed to wc_AesSetKeyDirect %d\n", ret); + } + else { + aes->left = AES_BLOCK_SIZE + 1; + tmp = wh_Client_AesCtr(ctx, aes, 1, plainIn, sizeof(plainIn), + cipher); + if (tmp != WH_ERROR_BADARGS) { + WH_ERROR_PRINT( + "AES-CTR: left > AES_BLOCK_SIZE should be BADARGS, " + "got %d\n", + tmp); + ret = -1; + } + } + (void)wc_AesFree(aes); + } + +#ifdef WOLFHSM_CFG_DMA + if (ret == 0) { + ret = wc_AesInit(aes, NULL, devId); + if (ret != 0) { + WH_ERROR_PRINT("Failed to wc_AesInit %d\n", ret); + } + else { + ret = wc_AesSetKeyDirect(aes, key, sizeof(key), iv, AES_ENCRYPTION); + if (ret != 0) { + WH_ERROR_PRINT("Failed to wc_AesSetKeyDirect %d\n", ret); + } + else { + aes->left = AES_BLOCK_SIZE + 1; + tmp = wh_Client_AesCtrDma(ctx, aes, 1, plainIn, + sizeof(plainIn), cipher); + if (tmp != WH_ERROR_BADARGS) { + WH_ERROR_PRINT( + "AES-CTR DMA: left > AES_BLOCK_SIZE should be " + "BADARGS, got %d\n", + tmp); + ret = -1; + } + } + (void)wc_AesFree(aes); + } + } +#endif /* WOLFHSM_CFG_DMA */ + + if (ret == 0) { + WH_TEST_PRINT("AES CTR left OOB DEVID=0x%X SUCCESS\n", devId); + } + return ret; +} +#endif /* WOLFSSL_AES_COUNTER */ + int whTest_CryptoAesKeyUsagePolicies(whClientContext* ctx) { int ret = 0; diff --git a/test-refactor/wh_test_list.c b/test-refactor/wh_test_list.c index 3d1d2b50c..81b941159 100644 --- a/test-refactor/wh_test_list.c +++ b/test-refactor/wh_test_list.c @@ -43,6 +43,7 @@ WH_TEST_DECL(whTest_CertVerify); WH_TEST_DECL(whTest_NvmOptional); WH_TEST_DECL(whTest_ClientCerts); WH_TEST_DECL(whTest_Crypto_Aes); +WH_TEST_DECL(whTest_CryptoAesCtrLeftOob); WH_TEST_DECL(whTest_CryptoAesKeyUsagePolicies); WH_TEST_DECL(whTest_Crypto_Cmac); WH_TEST_DECL(whTest_Crypto_Curve25519); @@ -74,6 +75,7 @@ const size_t whTestsServerCount = sizeof(whTestsServer) / sizeof(whTestsServer[0 const whTestCase whTestsClient[] = { { "whTest_ClientCerts", whTest_ClientCerts }, { "whTest_Crypto_Aes", whTest_Crypto_Aes }, + { "whTest_CryptoAesCtrLeftOob", whTest_CryptoAesCtrLeftOob }, { "whTest_CryptoAesKeyUsagePolicies", whTest_CryptoAesKeyUsagePolicies }, { "whTest_Crypto_Cmac", whTest_Crypto_Cmac }, { "whTest_Crypto_Curve25519", whTest_Crypto_Curve25519 }, From 15ec0436a76504d716e6eeb2eea058054c0dd5a4 Mon Sep 17 00:00:00 2001 From: Hideki Miyazaki Date: Fri, 12 Jun 2026 06:44:02 +0900 Subject: [PATCH 3/3] Changed the test to static --- test-refactor/client-server/wh_test_crypto_aes.c | 5 ++++- test-refactor/wh_test_list.c | 2 -- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/test-refactor/client-server/wh_test_crypto_aes.c b/test-refactor/client-server/wh_test_crypto_aes.c index 76b114eec..78c7edece 100644 --- a/test-refactor/client-server/wh_test_crypto_aes.c +++ b/test-refactor/client-server/wh_test_crypto_aes.c @@ -448,7 +448,7 @@ static int whTest_CryptoAesCbcStreaming(whClientContext* ctx) * wc_AesCtrEncrypt indexes aes->tmp via AES_BLOCK_SIZE - aes->left; an * oversized value would cause an out-of-bounds read disclosing server-side * memory across the HSM trust boundary. */ -int whTest_CryptoAesCtrLeftOob(whClientContext* ctx) +static int whTest_CryptoAesCtrLeftOob(whClientContext* ctx) { int devId = WH_DEV_ID; int ret = 0; @@ -1110,6 +1110,9 @@ int whTest_Crypto_Aes(whClientContext* ctx) /* CBC streaming drives the request/response API directly (INVALID_DEVID), * so it is not devId-routed -- run it once. */ WH_TEST_RETURN_ON_FAIL(whTest_CryptoAesCbcStreaming(ctx)); +#endif +#ifdef WOLFSSL_AES_COUNTER + WH_TEST_RETURN_ON_FAIL(whTest_CryptoAesCtrLeftOob(ctx)); #endif /* TODO: port legacy AES async + DMA-async coverage (comm-buffer & DMA, round-trip & KAT) -- the only remaining legacy crypto parity gap; deferred to follow-up PR. */ return 0; diff --git a/test-refactor/wh_test_list.c b/test-refactor/wh_test_list.c index 81b941159..3d1d2b50c 100644 --- a/test-refactor/wh_test_list.c +++ b/test-refactor/wh_test_list.c @@ -43,7 +43,6 @@ WH_TEST_DECL(whTest_CertVerify); WH_TEST_DECL(whTest_NvmOptional); WH_TEST_DECL(whTest_ClientCerts); WH_TEST_DECL(whTest_Crypto_Aes); -WH_TEST_DECL(whTest_CryptoAesCtrLeftOob); WH_TEST_DECL(whTest_CryptoAesKeyUsagePolicies); WH_TEST_DECL(whTest_Crypto_Cmac); WH_TEST_DECL(whTest_Crypto_Curve25519); @@ -75,7 +74,6 @@ const size_t whTestsServerCount = sizeof(whTestsServer) / sizeof(whTestsServer[0 const whTestCase whTestsClient[] = { { "whTest_ClientCerts", whTest_ClientCerts }, { "whTest_Crypto_Aes", whTest_Crypto_Aes }, - { "whTest_CryptoAesCtrLeftOob", whTest_CryptoAesCtrLeftOob }, { "whTest_CryptoAesKeyUsagePolicies", whTest_CryptoAesKeyUsagePolicies }, { "whTest_Crypto_Cmac", whTest_Crypto_Cmac }, { "whTest_Crypto_Curve25519", whTest_Crypto_Curve25519 },