diff --git a/wolfcrypt/src/dh.c b/wolfcrypt/src/dh.c index 849ff81025..26166c7310 100644 --- a/wolfcrypt/src/dh.c +++ b/wolfcrypt/src/dh.c @@ -1122,13 +1122,24 @@ static int GeneratePrivateDh186(DhKey* key, WC_RNG* rng, byte* priv, byte cBuf[DH_MAX_SIZE + 64 / WOLFSSL_BIT_SIZE]; #endif - /* Parameters validated in calling functions. */ + /* Pointer parameters validated by the public entry wc_DhGenerateKeyPair. */ if (mp_iszero(&key->q) == MP_YES) { WOLFSSL_MSG("DH q parameter needed for FIPS 186-4 key generation"); return BAD_FUNC_ARG; } + /* Bound *privSz so cSz (= *privSz + 8) cannot exceed the cBuf capacity. + * Note: DH_MAX_SIZE is documented as a bit count, but the cBuf declaration + * above uses it directly as a byte count (cBuf is DH_MAX_SIZE + 8 bytes). + * This check matches that convention so *privSz (in bytes) is bounded by + * the actual byte capacity of cBuf. The same bound is applied to the + * WOLFSSL_SMALL_STACK path to avoid unbounded heap allocation. */ + if (*privSz > DH_MAX_SIZE) { + WOLFSSL_MSG("DH private key size exceeds DH_MAX_SIZE"); + return BAD_FUNC_ARG; + } + qSz = (word32)mp_unsigned_bin_size(&key->q); pSz = (word32)mp_unsigned_bin_size(&key->p); diff --git a/wolfcrypt/src/sp_int.c b/wolfcrypt/src/sp_int.c index 19d64a13c1..05c78c8718 100644 --- a/wolfcrypt/src/sp_int.c +++ b/wolfcrypt/src/sp_int.c @@ -6140,14 +6140,17 @@ int sp_leading_bit(const sp_int* a) int sp_set_bit(sp_int* a, int i) { int err = MP_OKAY; - /* Get index of word to set. */ - sp_size_t w = (sp_size_t)(i >> SP_WORD_SHIFT); + /* Compute word index in full int width so that bit indices large enough + * to make the word index overflow sp_size_t are caught by the bounds + * check below rather than wrapping. */ + int wi = (i < 0) ? 0 : (i >> SP_WORD_SHIFT); /* Check for valid number and space for bit. */ - if ((a == NULL) || (i < 0) || (w >= a->size)) { + if ((a == NULL) || (i < 0) || (wi >= (int)a->size)) { err = MP_VAL; } if (err == MP_OKAY) { + sp_size_t w = (sp_size_t)wi; /* Amount to shift up to set bit in word. */ unsigned int s = (unsigned int)(i & (SP_WORD_SIZE - 1)); unsigned int j; @@ -8621,15 +8624,16 @@ void sp_rshd(sp_int* a, int c) { /* Do shift if we have an SP int. */ if ((a != NULL) && (c > 0)) { - /* Make zero if shift removes all digits. */ - if ((sp_size_t)c >= a->used) { + /* Compare c in int width to avoid narrowing to sp_size_t (which can + * be word16) before the bounds check. */ + if (c >= (int)a->used) { _sp_zero(a); } else { sp_size_t i; /* Update used digits count. */ - a->used = (sp_size_t)(a->used - c); + a->used = (sp_size_t)((int)a->used - c); /* Move digits down. */ for (i = 0; i < a->used; i++, c++) { a->dp[i] = a->dp[c]; @@ -8651,21 +8655,23 @@ void sp_rshd(sp_int* a, int c) int sp_rshb(const sp_int* a, int n, sp_int* r) { int err = MP_OKAY; - /* Number of digits to shift down. */ - sp_size_t i; + /* Compute the digit-shift count in full int width to avoid wrapping + * when n is large enough that the count would exceed sp_size_t range. */ + int ni = (n < 0) ? 0 : (n >> SP_WORD_SHIFT); if ((a == NULL) || (n < 0)) { err = MP_VAL; } /* Handle case where shifting out all digits. */ - else if ((i = (sp_size_t)(n >> SP_WORD_SHIFT)) >= a->used) { + else if (ni >= (int)a->used) { _sp_zero(r); } /* Change callers when more error cases returned. */ - else if ((err == MP_OKAY) && (a->used - i > r->size)) { + else if ((err == MP_OKAY) && ((int)a->used - ni > (int)r->size)) { err = MP_VAL; } else if (err == MP_OKAY) { + sp_size_t i = (sp_size_t)ni; sp_size_t j; /* Number of bits to shift in digits. */ @@ -14914,16 +14920,25 @@ int sp_div_2d(const sp_int* a, int e, sp_int* r, sp_int* rem) int sp_mod_2d(const sp_int* a, int e, sp_int* r) { int err = MP_OKAY; - sp_size_t digits = (sp_size_t)((e + SP_WORD_SIZE - 1) >> SP_WORD_SHIFT); + /* Compute digit count in full int width. Decompose to avoid signed + * overflow if e is near INT_MAX: (e + SP_WORD_SIZE - 1) >> SHIFT is + * equivalent to (e >> SHIFT) + (e has remainder ? 1 : 0). */ + int digits_full = 0; + sp_size_t digits = 0; if ((a == NULL) || (r == NULL) || (e < 0)) { err = MP_VAL; } - if ((err == MP_OKAY) && (digits > r->size)) { - err = MP_VAL; + if (err == MP_OKAY) { + digits_full = (e >> SP_WORD_SHIFT) + + (((e & (SP_WORD_SIZE - 1)) != 0) ? 1 : 0); + if (digits_full > (int)r->size) { + err = MP_VAL; + } } if (err == MP_OKAY) { + digits = (sp_size_t)digits_full; /* Copy a into r if not same pointer. */ if (a != r) { sp_size_t cnt = (a->used < digits) ? a->used : digits; diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index 6725de6aca..e225347f56 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -29605,6 +29605,27 @@ static wc_test_ret_t dh_fips_generate_test(WC_RNG *rng) if (ret != 0) ERROR_OUT(WC_TEST_RET_ENC_EC(ret), exit_gen_test); +#if !defined(WOLFSSL_NO_DH186) && !defined(HAVE_SELFTEST) && \ + !defined(HAVE_FIPS) + /* Regression: an oversized *privSz must be rejected before + * GeneratePrivateDh186 writes (*privSz + 8) bytes of RNG output into the + * stack-allocated cBuf (sized DH_MAX_SIZE + 8 in non-WOLFSSL_SMALL_STACK + * builds). The key still has q set here, so the call dispatches through + * GeneratePrivateDh186. Only exercised when the local src/dh.c bound + * check is in play (not HAVE_SELFTEST / HAVE_FIPS builds, which use + * separate validated modules). */ + { + word32 hugePrivSz = (word32)DH_MAX_SIZE + 1; + word32 outPubSz = sizeof(pub); + ret = wc_DhGenerateKeyPair(key, rng, priv, &hugePrivSz, pub, &outPubSz); + #if defined(WOLFSSL_ASYNC_CRYPT) + ret = wc_AsyncWait(ret, &key->asyncDev, WC_ASYNC_FLAG_NONE); + #endif + if (ret != WC_NO_ERR_TRACE(BAD_FUNC_ARG)) + ERROR_OUT(WC_TEST_RET_ENC_EC(ret), exit_gen_test); + } +#endif /* !WOLFSSL_NO_DH186 && !HAVE_SELFTEST && !HAVE_FIPS */ + wc_FreeDhKey(key); ret = wc_InitDhKey_ex(key, HEAP_HINT, devId); if (ret != 0)