Skip to content

Commit ff34b2d

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 ff34b2d

1 file changed

Lines changed: 15 additions & 4 deletions

File tree

wolfcrypt/src/wc_mldsa.c

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5612,13 +5612,21 @@ 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+
/* The whole expression is computed in word64 with a single conversion
5625+
* back: shifts on word64 avoid the signed-shift UB, |t << 23| < 2^54
5626+
* keeps every term exact, so the one conversion to sword64 is
5627+
* value-preserving ahead of the arithmetic >> 32. */
5628+
return (sword32)((sword64)((word64)a - ((word64)t << 23) +
5629+
((word64)t << 13) - (word64)t) >> 32);
56225630
#endif
56235631
}
56245632

@@ -5642,7 +5650,10 @@ static sword32 mldsa_red(sword32 a)
56425650
#ifndef MLDSA_MUL_Q_SLOW
56435651
return (sword32)(a - (t * MLDSA_Q));
56445652
#else
5645-
return (sword32)(a - (t << 23) + (t << 13) - t);
5653+
/* Shifts are done on word32: t is routinely negative and left-shifting
5654+
* a negative signed value is UB. Result is taken mod 2^32. */
5655+
return (sword32)((word32)a - ((word32)t << 23) + ((word32)t << 13) -
5656+
(word32)t);
56465657
#endif
56475658
}
56485659
#endif

0 commit comments

Comments
 (0)