Reject zero modulus in mulmod/sqrmod

[MarkAtwood]

Fixes #11063.

_sp_sqrmod() and _sp_mulmod_tmp() short-circuit on a zero operand and store zero without checking the modulus, while the non-aliased paths reject a zero modulus through sp_mod(). So the return value depended on whether the caller aliased the result onto the modulus:

mp_sqrmod(a, m, r)  a=0, m=0, r distinct  ->  MP_VAL
mp_sqrmod(a, m, m)  a=0, m=0              ->  MP_OKAY, result 0

mp_mulmod behaves the same way through _sp_mulmod_tmp(). Both public wrappers already document MP_VAL for a zero modulus (sp_sqrmod and sp_mulmod), so the aliased path did not match its documented contract.

The check goes in the zero-operand branch of each helper rather than in the public wrappers, so the non-zero path keeps getting it from sp_mod() as before.

Other backends

• tfm.c (fp_mulmod, fp_sqrmod) and integer.c (mp_mulmod, mp_sqrmod) have no zero-operand short-circuit and always reduce through fp_div/mp_div, which reject a zero divisor. Already correct in both aliasings, unchanged here.
• hw_mulmod() and hw_sqrmod() in port/maxim/max3266x.c carry the identical pattern, and wolfmath.h maps mp_mulmod/mp_sqrmod onto them when WOLFSSL_USE_HW_MP is defined. They get the same check. That hunk is by inspection only — building the port needs the Maxim SDK, so it has not been compiled or run. Happy to drop it into a separate PR if you would rather someone with the hardware take it.

Behavior change

The only production caller reaching the changed branch is wc_CheckRsaKey(), at rsa.c:927 and rsa.c:950, which call mp_mulmod(&key->dP, &key->e, tmp, tmp) with the result aliased onto the modulus. tmp is p-1/q-1, non-zero for any real key. For a malformed key with p == 1 the call now returns MP_VAL and the key is rejected as MP_EXPTMOD_E, where previously it returned MP_OKAY with tmp = 0 and was rejected by the following !mp_isone(tmp) check. Same outcome, explicit rather than incidental.

Testing

Extends the existing aliasing test in mp_test_mulmod_sqrmod() to cover a zero modulus in both arrangements, including both halves of the _sp_mulmod_tmp() zero-operand disjunction. The test fails before the sp_int.c change and passes after.

Note that mp_test() is gated behind WOLFSSL_PUBLIC_MP, so a default ./configure && make check skips the whole mp suite. Verified with CFLAGS=-DWOLFSSL_PUBLIC_MP. CI covers it via cmake.yml and os-check-linux.json.

Found via OSS-Fuzz 513887571 (cryptofuzz BignumCalc SqrMod).

[Copilot]

🟢 Ready to approve

The change is narrowly scoped, aligns behavior with the documented API contract, and is backed by targeted regression tests for the previously inconsistent aliasing cases.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

This PR fixes an inconsistency in the SP math mulmod/sqrmod helpers where a zero operand could short-circuit to a zero result without validating that the modulus is non-zero, making the return code depend on whether the result aliases the modulus.

Changes:

• Add explicit m == 0 validation in the zero-operand short-circuit paths of _sp_mulmod_tmp() and _sp_sqrmod() so aliasing and non-aliasing cases return MP_VAL consistently.
• Extend mp_test_mulmod_sqrmod() to cover zero-modulus behavior for both aliasing and non-aliasing arrangements (and both halves of the _sp_mulmod_tmp() zero-operand disjunction).
• Apply the same zero-modulus validation to the Maxim MAX3266x hardware backend’s zero-operand short-circuits for mulmod/sqrmod.

File summaries

File	Description
wolfcrypt/test/test.c	Adds regression tests ensuring zero modulus returns MP_VAL regardless of result aliasing in mulmod/sqrmod.
wolfcrypt/src/sp_int.c	Fixes SP math helper short-circuit to reject zero modulus in aliasing paths, aligning with the documented contract.
wolfcrypt/src/port/maxim/max3266x.c	Ensures hardware mulmod/sqrmod short-circuits also reject a zero modulus instead of returning a zero result.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 0
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[github-actions]

MemBrowse Memory Report

gcc-arm-cortex-m0plus

• FLASH: .text +4 B (+0.0%, 64,259 B / 262,144 B, total: 25% used)

gcc-arm-cortex-m3

• FLASH: .text +4 B (+0.0%, 122,947 B / 262,144 B, total: 47% used)

gcc-arm-cortex-m4-openssl-compat

• FLASH: .text +64 B (+0.0%, 772,908 B / 1,048,576 B, total: 74% used)

linuxkm-standard

• Data: __patchable_function_entries +8 B (+0.0%, 49,232 B)

stm32-sim-stm32h753

• FLASH: .text +64 B (+0.0%, 185,748 B / 2,097,152 B, total: 9% used)
  No memory changes detected for:
• 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
• linuxkm-pie