From c119a21931082a5935ab8d995b1806b10668024b Mon Sep 17 00:00:00 2001 From: Sean Parkinson Date: Thu, 16 Apr 2026 19:01:53 +1000 Subject: [PATCH] SP int: fixes from review by Claude MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. sp_cond_swap_ct_ex (line ~5524) — XOR typo: b->sign ^= b->sign always zeroed the sign. Fixed to b->sign ^= t->sign to correctly swap signs. 2. sp_mod_d (line ~7271) — Negative modulo correction was applied even when the remainder was 0. Added (*r != 0) guard to avoid producing d instead of 0. 3. sp_lshb (line ~8444) — Left-shift size check was off. Refactored to correctly distinguish between pure-digit shifts and bit-within-digit shifts when checking if the result fits, using separate overflow checks for each case. 4. _sp_mulmod_tmp (line ~12160) — Zero inputs caused an allocation of size 0, which is problematic. Added an early path: if either operand is zero, set result to zero and skip the allocation/multiply entirely. 5. sp_mod_2d — copy path (line ~14762) — XMEMCPY copied digits * SP_WORD_SIZEOF bytes but a may have fewer than digits used digits. Fixed to copy min(a->used, digits) digits to avoid reading uninitialized memory. 6. sp_mod_2d — negation loop (line ~14782) — Negation loop iterated over r->used, which could exceed digits. Fixed to loop over min(r->used, digits). 7. _sp_sqrmod (line ~17314) — Same zero-input issue as _sp_mulmod_tmp. Added early zero path to skip the allocation/squaring when input is zero. 8. sp_lcm (line ~19838) — Typo in sign check: b->sign >= MP_NEG (comparing against a value that is 1, so >= 1 would also match MP_ZPOS) changed to b->sign == MP_NEG. --- wolfcrypt/src/sp_int.c | 86 +++++++++++++++++++++++++----------------- 1 file changed, 52 insertions(+), 34 deletions(-) diff --git a/wolfcrypt/src/sp_int.c b/wolfcrypt/src/sp_int.c index 255b43f001..b6ce7d1a34 100644 --- a/wolfcrypt/src/sp_int.c +++ b/wolfcrypt/src/sp_int.c @@ -8546,14 +8546,17 @@ static int sp_lshb(sp_int* a, int n) if (a->used != 0) { /* Calculate number of digits to shift. */ sp_size_t s = (sp_size_t)n >> SP_WORD_SHIFT; + /* Get count of bits to move in digit. */ + n &= (int)SP_WORD_MASK; /* Ensure number has enough digits for result. */ - if (a->used + s >= a->size) { + if ((n != 0) && (a->used + s >= a->size)) { + err = MP_VAL; + } + else if ((s > 0) && (a->used + s > a->size)) { err = MP_VAL; } if (err == MP_OKAY) { - /* Get count of bits to move in digit. */ - n &= (int)SP_WORD_MASK; /* Check whether this is a complicated case. */ if (n != 0) { unsigned int i; @@ -12258,24 +12261,30 @@ static int _sp_mulmod_tmp(const sp_int* a, const sp_int* b, const sp_int* m, sp_int* r) { int err = MP_OKAY; - /* Create temporary for multiplication result. */ - DECL_SP_INT(t, a->used + b->used); - ALLOC_SP_INT(t, a->used + b->used, err, NULL); - if (err == MP_OKAY) { - err = sp_init_size(t, (sp_size_t)(a->used + b->used)); + if (sp_iszero(a) || sp_iszero(b)) { + _sp_zero(r); } + else { + /* Create temporary for multiplication result. */ + DECL_SP_INT(t, a->used + b->used); - /* Multiply and reduce. */ - if (err == MP_OKAY) { - err = sp_mul(a, b, t); - } - if (err == MP_OKAY) { - err = sp_mod(t, m, r); - } + ALLOC_SP_INT(t, a->used + b->used, err, NULL); + if (err == MP_OKAY) { + err = sp_init_size(t, (sp_size_t)(a->used + b->used)); + } - /* Dispose of an allocated SP int. */ - FREE_SP_INT(t, NULL); + /* Multiply and reduce. */ + if (err == MP_OKAY) { + err = sp_mul(a, b, t); + } + if (err == MP_OKAY) { + err = sp_mod(t, m, r); + } + + /* Dispose of an allocated SP int. */ + FREE_SP_INT(t, NULL); + } return err; } @@ -14851,7 +14860,8 @@ int sp_mod_2d(const sp_int* a, int e, sp_int* r) if (err == MP_OKAY) { /* Copy a into r if not same pointer. */ if (a != r) { - XMEMCPY(r->dp, a->dp, digits * (word32)SP_WORD_SIZEOF); + sp_size_t cnt = (a->used < digits) ? a->used : digits; + XMEMCPY(r->dp, a->dp, cnt * (word32)SP_WORD_SIZEOF); r->used = a->used; #ifdef WOLFSSL_SP_INT_NEGATIVE r->sign = a->sign; @@ -14870,9 +14880,10 @@ int sp_mod_2d(const sp_int* a, int e, sp_int* r) if (a->sign == MP_NEG) { unsigned int i; sp_int_digit carry = 0; + sp_size_t cnt = (r->used < digits) ? r->used : digits; /* Negate value. */ - for (i = 0; i < r->used; i++) { + for (i = 0; i < cnt; i++) { sp_int_digit next = r->dp[i] > 0; r->dp[i] = (sp_int_digit)0 - r->dp[i] - carry; carry |= next; @@ -17401,24 +17412,31 @@ int sp_sqr(const sp_int* a, sp_int* r) static int _sp_sqrmod(const sp_int* a, const sp_int* m, sp_int* r) { int err = MP_OKAY; - /* Create temporary for multiplication result. */ - DECL_SP_INT(t, a->used * 2); - ALLOC_SP_INT(t, a->used * 2, err, NULL); - if (err == MP_OKAY) { - err = sp_init_size(t, a->used * 2U); + if (sp_iszero(a)) { + _sp_zero(r); + } + else { + /* Create temporary for multiplication result. */ + DECL_SP_INT(t, a->used * 2); + + ALLOC_SP_INT(t, a->used * 2, err, NULL); + if (err == MP_OKAY) { + err = sp_init_size(t, a->used * 2U); + } + + /* Square and reduce. */ + if (err == MP_OKAY) { + err = sp_sqr(a, t); + } + if (err == MP_OKAY) { + err = sp_mod(t, m, r); + } + + /* Dispose of an allocated SP int. */ + FREE_SP_INT(t, NULL); } - /* Square and reduce. */ - if (err == MP_OKAY) { - err = sp_sqr(a, t); - } - if (err == MP_OKAY) { - err = sp_mod(t, m, r); - } - - /* Dispose of an allocated SP int. */ - FREE_SP_INT(t, NULL); return err; }