Skip to content

Commit c119a21

Browse files
committed
SP int: fixes from review by Claude
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.
1 parent f086e91 commit c119a21

1 file changed

Lines changed: 50 additions & 32 deletions

File tree

wolfcrypt/src/sp_int.c

Lines changed: 50 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -8546,14 +8546,17 @@ static int sp_lshb(sp_int* a, int n)
85468546
if (a->used != 0) {
85478547
/* Calculate number of digits to shift. */
85488548
sp_size_t s = (sp_size_t)n >> SP_WORD_SHIFT;
8549+
/* Get count of bits to move in digit. */
8550+
n &= (int)SP_WORD_MASK;
85498551

85508552
/* Ensure number has enough digits for result. */
8551-
if (a->used + s >= a->size) {
8553+
if ((n != 0) && (a->used + s >= a->size)) {
8554+
err = MP_VAL;
8555+
}
8556+
else if ((s > 0) && (a->used + s > a->size)) {
85528557
err = MP_VAL;
85538558
}
85548559
if (err == MP_OKAY) {
8555-
/* Get count of bits to move in digit. */
8556-
n &= (int)SP_WORD_MASK;
85578560
/* Check whether this is a complicated case. */
85588561
if (n != 0) {
85598562
unsigned int i;
@@ -12258,24 +12261,30 @@ static int _sp_mulmod_tmp(const sp_int* a, const sp_int* b, const sp_int* m,
1225812261
sp_int* r)
1225912262
{
1226012263
int err = MP_OKAY;
12261-
/* Create temporary for multiplication result. */
12262-
DECL_SP_INT(t, a->used + b->used);
1226312264

12264-
ALLOC_SP_INT(t, a->used + b->used, err, NULL);
12265-
if (err == MP_OKAY) {
12266-
err = sp_init_size(t, (sp_size_t)(a->used + b->used));
12265+
if (sp_iszero(a) || sp_iszero(b)) {
12266+
_sp_zero(r);
1226712267
}
12268+
else {
12269+
/* Create temporary for multiplication result. */
12270+
DECL_SP_INT(t, a->used + b->used);
1226812271

12269-
/* Multiply and reduce. */
12270-
if (err == MP_OKAY) {
12271-
err = sp_mul(a, b, t);
12272-
}
12273-
if (err == MP_OKAY) {
12274-
err = sp_mod(t, m, r);
12275-
}
12272+
ALLOC_SP_INT(t, a->used + b->used, err, NULL);
12273+
if (err == MP_OKAY) {
12274+
err = sp_init_size(t, (sp_size_t)(a->used + b->used));
12275+
}
1227612276

12277-
/* Dispose of an allocated SP int. */
12278-
FREE_SP_INT(t, NULL);
12277+
/* Multiply and reduce. */
12278+
if (err == MP_OKAY) {
12279+
err = sp_mul(a, b, t);
12280+
}
12281+
if (err == MP_OKAY) {
12282+
err = sp_mod(t, m, r);
12283+
}
12284+
12285+
/* Dispose of an allocated SP int. */
12286+
FREE_SP_INT(t, NULL);
12287+
}
1227912288

1228012289
return err;
1228112290
}
@@ -14851,7 +14860,8 @@ int sp_mod_2d(const sp_int* a, int e, sp_int* r)
1485114860
if (err == MP_OKAY) {
1485214861
/* Copy a into r if not same pointer. */
1485314862
if (a != r) {
14854-
XMEMCPY(r->dp, a->dp, digits * (word32)SP_WORD_SIZEOF);
14863+
sp_size_t cnt = (a->used < digits) ? a->used : digits;
14864+
XMEMCPY(r->dp, a->dp, cnt * (word32)SP_WORD_SIZEOF);
1485514865
r->used = a->used;
1485614866
#ifdef WOLFSSL_SP_INT_NEGATIVE
1485714867
r->sign = a->sign;
@@ -14870,9 +14880,10 @@ int sp_mod_2d(const sp_int* a, int e, sp_int* r)
1487014880
if (a->sign == MP_NEG) {
1487114881
unsigned int i;
1487214882
sp_int_digit carry = 0;
14883+
sp_size_t cnt = (r->used < digits) ? r->used : digits;
1487314884

1487414885
/* Negate value. */
14875-
for (i = 0; i < r->used; i++) {
14886+
for (i = 0; i < cnt; i++) {
1487614887
sp_int_digit next = r->dp[i] > 0;
1487714888
r->dp[i] = (sp_int_digit)0 - r->dp[i] - carry;
1487814889
carry |= next;
@@ -17401,24 +17412,31 @@ int sp_sqr(const sp_int* a, sp_int* r)
1740117412
static int _sp_sqrmod(const sp_int* a, const sp_int* m, sp_int* r)
1740217413
{
1740317414
int err = MP_OKAY;
17404-
/* Create temporary for multiplication result. */
17405-
DECL_SP_INT(t, a->used * 2);
1740617415

17407-
ALLOC_SP_INT(t, a->used * 2, err, NULL);
17408-
if (err == MP_OKAY) {
17409-
err = sp_init_size(t, a->used * 2U);
17416+
if (sp_iszero(a)) {
17417+
_sp_zero(r);
1741017418
}
17419+
else {
17420+
/* Create temporary for multiplication result. */
17421+
DECL_SP_INT(t, a->used * 2);
1741117422

17412-
/* Square and reduce. */
17413-
if (err == MP_OKAY) {
17414-
err = sp_sqr(a, t);
17415-
}
17416-
if (err == MP_OKAY) {
17417-
err = sp_mod(t, m, r);
17423+
ALLOC_SP_INT(t, a->used * 2, err, NULL);
17424+
if (err == MP_OKAY) {
17425+
err = sp_init_size(t, a->used * 2U);
17426+
}
17427+
17428+
/* Square and reduce. */
17429+
if (err == MP_OKAY) {
17430+
err = sp_sqr(a, t);
17431+
}
17432+
if (err == MP_OKAY) {
17433+
err = sp_mod(t, m, r);
17434+
}
17435+
17436+
/* Dispose of an allocated SP int. */
17437+
FREE_SP_INT(t, NULL);
1741817438
}
1741917439

17420-
/* Dispose of an allocated SP int. */
17421-
FREE_SP_INT(t, NULL);
1742217440
return err;
1742317441
}
1742417442

0 commit comments

Comments
 (0)