Repository navigation
ML-DSA signing counter wraps around as UINT16 #55
Description
Hi, the rejection sampling counter k in the ML-DSA signing loop is declared as UINT16 (mldsa.c:461). For ML-DSA-87 (nCols=7), after 9362 iterations k wraps from 65534 back to 5:
UINT16 k = 0; // line 461
while( TRUE ) {
SymCryptMlDsaExpandMask(..., k, ...);
k += (UINT16) pParams->nCols; // line 475: 65534 + 7 = 5 (wrapped)After wrap, ExpandMask at line 1082 produces identical SHAKE inputs (SYMCRYPT_STORE_LSBFIRST16(seedSuffix, counter + i)), which means nonce reuse in the mask polynomial generation.
I want to note that the NIST reference implementation has the same issue — dilithium/ref/sign.c:97 uses uint16_t nonce with the same arithmetic. So SymCrypt is faithfully following the reference. FIPS 204 describes kappa as an "integer" without specifying width.
The probability of actually hitting 9362 iterations with a valid key is astronomically low (~2^{-23400} for ML-DSA-87), so this is theoretical. But widening to UINT32 would eliminate the possibility entirely:
- UINT16 k = 0;
+ UINT32 k = 0;This might be worth raising with NIST as well, since the root cause is in the reference code.
Activity
mlindgren commentedon Mar 30, 2026
Hi Sunwoo Lee (@programsurf), good find, and thanks for reporting this! We have an open bug internally to add upper bounds to the number of loop iterations per Appendix C in FIPS 204. This is optional in the spec, but is good defense-in-depth against the possibility of an infinite loop when importing a malformed key. Implementing that change would also fix this issue, since the suggested loop bounds are substantially lower than 9,362 iterations.
I'll prioritize fixing this soon. We should be publishing the latest version to GitHub soon as well, though this change probably won't make it into that version, so it will be in the next version after that.
programsurf commentedon Mar 31, 2026
Thanks for the quick response and for prioritizing the fix! When this is addressed, could I be credited in the release notes or changelog? For attribution: Sunwoo Lee, Woohyun Choi, and Seunghyun Yoon, Korea Institute of Energy Technology (KENTECH).
Also, I'm planning to report the same issue to the NIST reference implementation as well, since the root cause originates there.
Edit: Seunghyun Yoon (@yoonsh) Woo-Hyun, Choi (@woohyunchoi-kentech)
mlindgren commentedon Mar 31, 2026
Sure, happy to credit you as the finder of the overflow issue. 🙂
Edit: after discussing with my colleagues, though, I do want to clarify the scope and severity of this issue in case my previous comments may have been misinterpreted. As you said, the probability of the overflow happening with random inputs is "astronomically low", so we do not consider this a vulnerability. But since we are already planning to add defense-in-depth for the potential of a DOS attack triggered by a maliciously crafted key, this will be fixed as well. There is currently no known exploit with maliciously crafted keys either, though, so this is purely about hardening.
programsurf commentedon Mar 31, 2026
Thanks for looking into this! Just to clarify — I understand this is a bug rather than a vulnerability, so no urgency intended. Happy to hear it'll be addressed alongside the Appendix C loop bounds work. Appreciate the transparency on the timeline!
Merged PR 15184356: Add upper bounds to probabilistic sampling loops …
mlindgren commentedon Jun 1, 2026
Fixed in 02c37be