Skip to content

Commit 08aaf00

Browse files
committed
fix: signed-shift UB in ML-DSA slow reductions
Under MLDSA_MUL_QINV_SLOW / MLDSA_MUL_Q_SLOW, mldsa_mont_red and mldsa_red left-shift signed operands that are routinely negative (NTT zetas), which is undefined behavior. Perform the shifts on word32/word64 and convert back; results are bit-identical. Only opt-in slow-multiply builds compile these paths. Reproduced with -fsanitize=shift --disable-intelasm; clean after this change.
1 parent 9d86960 commit 08aaf00

1 file changed

Lines changed: 13 additions & 4 deletions

File tree

wolfcrypt/src/wc_mldsa.c

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5612,13 +5612,19 @@ static sword32 mldsa_mont_red(sword64 a)
56125612
#ifndef MLDSA_MUL_QINV_SLOW
56135613
sword64 t = (sword32)((sword32)a * (sword32)MLDSA_QINV);
56145614
#else
5615-
sword64 t = (sword32)((sword32)a + (sword32)((sword32)a << 13) -
5616-
(sword32)((sword32)a << 23) + (sword32)((sword32)a << 26));
5615+
/* Shifts are done on word32: a is routinely negative and left-shifting
5616+
* a negative signed value is UB. Only the low 32 bits are kept, so
5617+
* unsigned wrap-around gives the identical result. */
5618+
sword64 t = (sword32)((word32)a + ((word32)a << 13) -
5619+
((word32)a << 23) + ((word32)a << 26));
56175620
#endif
56185621
#ifndef MLDSA_MUL_Q_SLOW
56195622
return (sword32)((a - ((sword32)t * (sword64)MLDSA_Q)) >> 32);
56205623
#else
5621-
return (sword32)((a - (t << 23) + (t << 13) - t) >> 32);
5624+
/* Shifts are done on word64 for the same reason; |t << 23| < 2^54 so
5625+
* the values are exact, not just congruent mod 2^64. */
5626+
return (sword32)((a - (sword64)((word64)t << 23) +
5627+
(sword64)((word64)t << 13) - t) >> 32);
56225628
#endif
56235629
}
56245630

@@ -5642,7 +5648,10 @@ static sword32 mldsa_red(sword32 a)
56425648
#ifndef MLDSA_MUL_Q_SLOW
56435649
return (sword32)(a - (t * MLDSA_Q));
56445650
#else
5645-
return (sword32)(a - (t << 23) + (t << 13) - t);
5651+
/* Shifts are done on word32: t is routinely negative and left-shifting
5652+
* a negative signed value is UB. Result is taken mod 2^32. */
5653+
return (sword32)((word32)a - ((word32)t << 23) + ((word32)t << 13) -
5654+
(word32)t);
56465655
#endif
56475656
}
56485657
#endif

0 commit comments

Comments
 (0)