Skip to content

Fixes in SP int and DH #10478

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
embhorn wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fixes in SP int and DH #10478

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8c85618
Fix SP int truncation issue
embhorn May 12, 2026
dc6db15
Check params to GeneratePrivateDh186
embhorn May 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion wolfcrypt/src/dh.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Comment thread
embhorn marked this conversation as resolved.
Comment thread
embhorn marked this conversation as resolved.
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);

Expand Down
41 changes: 28 additions & 13 deletions wolfcrypt/src/sp_int.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Comment thread
embhorn marked this conversation as resolved.
/* Move digits down. */
for (i = 0; i < a->used; i++, c++) {
a->dp[i] = a->dp[c];
Expand All @@ -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. */
Expand Down Expand Up @@ -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);
Comment thread
embhorn marked this conversation as resolved.
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;
Expand Down
21 changes: 21 additions & 0 deletions wolfcrypt/test/test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading