[yosuke-wolfssl]

Problem

sc_reduce() and sc_muladd() in the C Ed25519 scalar code can return a value outside [0, l). The last fold of the reduction leaves a result that is either l too low or l too high, and nothing corrects it:

Inputs are SHA-512 outputs and the clamped private scalar, so the odds are about 2^-127 per operation and no party can steer them. Although it breaks the documented s mod l contract.

Fix (wolfcrypt/src/ge_operations.c)

After the final carry chain, without branching on the value:

• All four functions add l back when the top limb is negative, using a sign mask, then repeat the carry chain.
• The __int128_t versions then subtract l and add it back under the same mask if the result goes negative, which maps [-l, 2l) to [0, l).

Values already in [0, l) come out unchanged. Closes f-17452.

Verification

• sc_reduce and sc_muladd are WOLFSSL_LOCAL, and no public API can steer an input into either range, so I checked them with an out-of-tree harness linked against the static library, compared with exact bignum % l. Both versions: 0 wrong out of about 330k vectors (edge values, in-range residues, uniform random, caller-shaped inputs, and targeted top-carry patterns). The unpatched code gets 24,557 of 59,991 targeted vectors wrong.
• testwolfcrypt and unit.test --group ed25519 pass, with and without NO_CURVED25519_128BIT. ASan + UBSan clean. GCC -Werror clean on Ubuntu 24.04.
• Ed25519 sign/verify throughput changes by -0.4% to +1.1%, within run-to-run noise.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

[yosuke-wolfssl]

@claude review

[github-actions]

MemBrowse Memory Report · Explore what changed →

gcc-arm-cortex-m4-openssl-compat

• FLASH: .text +640 B (+0.1%, 802,148 B / 1,048,576 B, total: 76% used)

zephyr-frdm_rw612

• FLASH: datas +16 B, log_const_area -8 B, rodata -8 B, text +3,688 B (+1.3%, 286,152 B / 67,108,864 B, total: 0% used)
• RAM: bss +49 B, datas +16 B (+0.1%, 58,423 B / 983,040 B, total: 6% used)

No memory changes detected for:

• gcc-arm-cortex-m0plus
• gcc-arm-cortex-m3
• gcc-arm-cortex-m4
• gcc-arm-cortex-m4-baremetal
• gcc-arm-cortex-m4-crypto-only
• gcc-arm-cortex-m4-dtls13
• gcc-arm-cortex-m4-min-ecc
• gcc-arm-cortex-m4-pkcs7
• gcc-arm-cortex-m4-pq
• gcc-arm-cortex-m4-rsa-only
• gcc-arm-cortex-m4-sp-math
• gcc-arm-cortex-m4-tls12
• gcc-arm-cortex-m4-tls13
• gcc-arm-cortex-m7
• gcc-arm-cortex-m7-pq
• gcc-arm-cortex-m7-tls13
• stm32-sim-stm32h753

---

_{Catch memory regressions in CI with MemBrowse. Free for open source.}

[claude]

I reviewed this PR and found no bugs. It changes Ed25519 scalar reduction in constant-time crypto code, and the 21-bit path's correctness depends on a range claim, so a maintainer familiar with this code should also take a look.

What was reviewed:

• The new correction steps in all four sc_reduce/sc_muladd variants. They reuse the ORDER_* and MASK_* constants already defined for each limb layout, and the 2^252 term goes into the top limb (1<<21 at t[11], 1<<42 at t[5]).
• The fixes are branch-free: each one builds a sign mask from the top limb and runs a full carry chain into it before the bytes are written out. The new mask locals are declared at the top of each function, as C90 requires.
• The 21-bit path adds l back only when the result is negative. That is enough only if its result can never be l or larger, and I didn't prove that range here. I also didn't run the author's test harness or the test suites.

Extended reasoning...

The change adds 122 lines to wolfcrypt/src/ge_operations.c. It puts a final reduction to [0, l) at the end of sc_reduce and sc_muladd, in both the 21-bit-limb and __int128 implementations. These functions sit on the Ed25519 signing path, so this is security-sensitive crypto code that has to stay constant-time. The new code follows the existing carry-chain pattern, but the 21-bit path only adds l back and never subtracts it. Whether that is enough depends on bounds the diff doesn't establish, which is why I'm recommending a human review rather than approving.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #12076

Scan targets checked: wolfcrypt-src, wolfcrypt-bugs
Coverage: 1 of 1 in-scope changed file(s) opened by the reviewer

Fenrir result: Approved ✅

No new issues found in the changed files.

Advisory only — this automated result does not count as a GitHub approval.

Review tier: Max