Skip to content

Commit c65e3e5

Browse files
authored
Merge pull request #9825 from embhorn/zd21240
Fix issue in TLS_hmac size calculation
2 parents 178f96c + cbe8dd5 commit c65e3e5

4 files changed

Lines changed: 94 additions & 7 deletions

File tree

src/tls.c

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1283,6 +1283,7 @@ int TLS_hmac(WOLFSSL* ssl, byte* digest, const byte* in, word32 sz, int padSz,
12831283
int ret = 0;
12841284
const byte* macSecret = NULL;
12851285
word32 hashSz = 0;
1286+
word32 totalSz = 0;
12861287

12871288
if (ssl == NULL)
12881289
return BAD_FUNC_ARG;
@@ -1294,11 +1295,23 @@ int TLS_hmac(WOLFSSL* ssl, byte* digest, const byte* in, word32 sz, int padSz,
12941295
hashSz = ssl->specs.hash_size;
12951296
#endif
12961297

1298+
/* Pre-compute sz + hashSz + padSz + 1 with overflow checking.
1299+
* Used by fuzzer callback and Hmac_UpdateFinal* in the verify path. */
1300+
if (verify && padSz >= 0) {
1301+
word32 hmacSz = 0;
1302+
if (!WC_SAFE_SUM_WORD32(sz, hashSz, hmacSz) ||
1303+
!WC_SAFE_SUM_WORD32(hmacSz, (word32)padSz, hmacSz) ||
1304+
!WC_SAFE_SUM_WORD32(hmacSz, 1, hmacSz)) {
1305+
return BUFFER_E;
1306+
}
1307+
totalSz = hmacSz;
1308+
}
1309+
12971310
#ifdef HAVE_FUZZER
12981311
/* Fuzz "in" buffer with sz to be used in HMAC algorithm */
12991312
if (ssl->fuzzerCb) {
13001313
if (verify && padSz >= 0) {
1301-
ssl->fuzzerCb(ssl, in, sz + hashSz + padSz + 1, FUZZ_HMAC,
1314+
ssl->fuzzerCb(ssl, in, totalSz, FUZZ_HMAC,
13021315
ssl->fuzzerCtx);
13031316
}
13041317
else {
@@ -1335,19 +1348,18 @@ int TLS_hmac(WOLFSSL* ssl, byte* digest, const byte* in, word32 sz, int padSz,
13351348
#ifdef HAVE_BLAKE2
13361349
if (wolfSSL_GetHmacType(ssl) == WC_HASH_TYPE_BLAKE2B) {
13371350
ret = Hmac_UpdateFinal(&hmac, digest, in,
1338-
sz + hashSz + (word32)padSz + 1, myInner, innerSz);
1351+
totalSz, myInner, innerSz);
13391352
}
13401353
else
13411354
#endif
13421355
{
13431356
ret = Hmac_UpdateFinal_CT(&hmac, digest, in,
1344-
(sz + hashSz + (word32)padSz + 1),
1357+
totalSz,
13451358
(int)hashSz, myInner, innerSz);
13461359

13471360
}
13481361
#else
1349-
ret = Hmac_UpdateFinal(&hmac, digest, in, sz + hashSz +
1350-
(word32)(padSz) + 1,
1362+
ret = Hmac_UpdateFinal(&hmac, digest, in, totalSz,
13511363
myInner, innerSz);
13521364
#endif
13531365
}

tests/api/test_hmac.c

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030

3131
#include <wolfssl/wolfcrypt/hmac.h>
3232
#include <wolfssl/wolfcrypt/types.h>
33+
#include <wolfssl/internal.h>
3334
#include <tests/api/api.h>
3435
#include <tests/api/test_hmac.h>
3536

@@ -681,3 +682,76 @@ int test_wc_Sha384HmacFinal(void)
681682
return EXPECT_RESULT();
682683
} /* END test_wc_Sha384HmacFinal */
683684

685+
/* Test for integer overflow in TLS_hmac size calculation (ZD #21240).
686+
*
687+
* TLS_hmac() computes sz + hashSz + padSz + 1 and passes the result to
688+
* Hmac_UpdateFinal / Hmac_UpdateFinal_CT. When sz (word32) is near
689+
* UINT32_MAX, the addition overflows and wraps to a small value, causing
690+
* the HMAC routines to operate on an undersized length. The fix adds
691+
* WC_SAFE_SUM_WORD32 overflow checks and returns BUFFER_E on overflow.
692+
*
693+
* This test calls through ssl->hmac (which points to TLS_hmac) with
694+
* values that trigger the overflow condition and verifies the function
695+
* correctly rejects them.
696+
*/
697+
int test_tls_hmac_size_overflow(void)
698+
{
699+
EXPECT_DECLS;
700+
#if !defined(NO_HMAC) && !defined(WOLFSSL_AEAD_ONLY) && !defined(NO_TLS) && \
701+
defined(NO_OLD_TLS) && !defined(NO_WOLFSSL_CLIENT)
702+
WOLFSSL_CTX* ctx = NULL;
703+
WOLFSSL* ssl = NULL;
704+
byte digest[WC_MAX_DIGEST_SIZE];
705+
byte dummy_in[64];
706+
707+
XMEMSET(dummy_in, 0xAA, sizeof(dummy_in));
708+
XMEMSET(digest, 0, sizeof(digest));
709+
710+
ctx = wolfSSL_CTX_new(wolfSSLv23_client_method());
711+
ExpectNotNull(ctx);
712+
ssl = wolfSSL_new(ctx);
713+
ExpectNotNull(ssl);
714+
715+
if (EXPECT_SUCCESS()) {
716+
ExpectNotNull(ssl->hmac);
717+
718+
/* Set a hash size so the verify path in TLS_hmac is exercised. */
719+
ssl->specs.hash_size = WC_SHA256_DIGEST_SIZE;
720+
721+
/* Overflow case 1: sz near UINT32_MAX, padSz pushes sum past limit.
722+
* (UINT32_MAX - 300) + 32 + 500 + 1 = UINT32_MAX + 233 -> wraps to 232
723+
*/
724+
ExpectIntEQ(ssl->hmac(ssl, digest, dummy_in,
725+
(word32)(WOLFSSL_MAX_32BIT - 300),
726+
500, /* padSz */
727+
application_data, 1, PEER_ORDER),
728+
WC_NO_ERR_TRACE(BUFFER_E));
729+
730+
/* Overflow case 2: padSz = 0, hashSz alone causes overflow.
731+
* (UINT32_MAX - 10) + 32 + 0 + 1 = UINT32_MAX + 23 -> wraps to 22
732+
*/
733+
ExpectIntEQ(ssl->hmac(ssl, digest, dummy_in,
734+
(word32)(WOLFSSL_MAX_32BIT - 10),
735+
0, /* padSz */
736+
application_data, 1, PEER_ORDER),
737+
WC_NO_ERR_TRACE(BUFFER_E));
738+
739+
/* Normal case: should NOT return BUFFER_E.
740+
* May fail for other reasons (no keys configured) but the overflow
741+
* check must not fire for small legitimate values.
742+
*/
743+
ExpectIntNE(ssl->hmac(ssl, digest, dummy_in,
744+
100,
745+
10, /* padSz */
746+
application_data, 1, PEER_ORDER),
747+
WC_NO_ERR_TRACE(BUFFER_E));
748+
}
749+
750+
wolfSSL_free(ssl);
751+
wolfSSL_CTX_free(ctx);
752+
wolfSSL_Cleanup();
753+
#endif /* !NO_HMAC && !WOLFSSL_AEAD_ONLY && !NO_TLS && NO_OLD_TLS &&
754+
* !NO_WOLFSSL_CLIENT */
755+
return EXPECT_RESULT();
756+
} /* END test_tls_hmac_size_overflow */
757+

tests/api/test_hmac.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ int test_wc_Sha256HmacFinal(void);
3939
int test_wc_Sha384HmacSetKey(void);
4040
int test_wc_Sha384HmacUpdate(void);
4141
int test_wc_Sha384HmacFinal(void);
42+
int test_tls_hmac_size_overflow(void);
4243

4344
#define TEST_HMAC_DECLS \
4445
TEST_DECL_GROUP("hmac", test_wc_Md5HmacSetKey), \
@@ -55,6 +56,7 @@ int test_wc_Sha384HmacFinal(void);
5556
TEST_DECL_GROUP("hmac", test_wc_Sha256HmacFinal), \
5657
TEST_DECL_GROUP("hmac", test_wc_Sha384HmacSetKey), \
5758
TEST_DECL_GROUP("hmac", test_wc_Sha384HmacUpdate), \
58-
TEST_DECL_GROUP("hmac", test_wc_Sha384HmacFinal)
59+
TEST_DECL_GROUP("hmac", test_wc_Sha384HmacFinal), \
60+
TEST_DECL_GROUP("hmac", test_tls_hmac_size_overflow)
5961

6062
#endif /* WOLFCRYPT_TEST_HMAC_H */

wolfcrypt/src/ecc.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1851,7 +1851,6 @@ int wc_ecc_curve_cache_init(void)
18511851
void wc_ecc_curve_cache_free(void)
18521852
{
18531853
int x;
1854-
18551854
/* free all ECC curve caches */
18561855
for (x = 0; x < (int)ECC_SET_COUNT; x++) {
18571856
#ifdef WOLFSSL_NO_MALLOC

0 commit comments

Comments
 (0)