From d0f4ad8cdaf5fbbec98889bfcfb3a04082ff6f94 Mon Sep 17 00:00:00 2001 From: Mamy Ratsimbazafy Date: Sun, 2 Jul 2023 17:15:02 +0200 Subject: [PATCH] Fix fuzz #1 failure: incorrect reduction of BigInt (#246) --- constantine/math/arithmetic/limbs_montgomery.nim | 8 +++++++- .../arithmetic/bigints_views.nim | 1 - tests/math_fields/t_io_fields.nim | 10 ++++++++++ 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/constantine/math/arithmetic/limbs_montgomery.nim b/constantine/math/arithmetic/limbs_montgomery.nim index 840c8b30..9a651466 100644 --- a/constantine/math/arithmetic/limbs_montgomery.nim +++ b/constantine/math/arithmetic/limbs_montgomery.nim @@ -609,7 +609,13 @@ func getMont*(r: var Limbs, a, M, r2modM: Limbs, ## Important: `r` is overwritten ## The result `r` buffer size MUST be at least the size of `M` buffer # Reference: https://eprint.iacr.org/2017/1057.pdf - mulMont(r, a, r2ModM, M, m0ninv, spareBits) + + # For conversion to a field element (in the Montgomery domain), we do not use the "no-carry" optimization: + # While Montgomery Reduction can map inputs [0, 4p²) -> [0, p) + # that range is not valid with the no-carry optimization, + # hence an unreduced input that uses 256-bit while prime is 254-bit + # can have an incorrect representation. + mulMont_FIPS(r, a, r2ModM, M, m0ninv, skipFinalSub = false) # Montgomery Modular Exponentiation # ------------------------------------------ diff --git a/constantine/math_arbitrary_precision/arithmetic/bigints_views.nim b/constantine/math_arbitrary_precision/arithmetic/bigints_views.nim index e7bae351..0c953f48 100644 --- a/constantine/math_arbitrary_precision/arithmetic/bigints_views.nim +++ b/constantine/math_arbitrary_precision/arithmetic/bigints_views.nim @@ -70,7 +70,6 @@ func powOddMod_vartime*( # if we use redc2xMont (a/R) and montgomery multiplication by R³ # For now, we call explicit reduction as it can handle all sizes. # TODO: explicit reduction uses constant-time division which is **very** expensive - # TODO: fix https://github.com/mratsim/constantine/issues/241 if a.len != M.len: let t = allocStackArray(SecretWord, L) t.LimbsViewMut.reduce(a.view(), aBits, M.view(), mBits) diff --git a/tests/math_fields/t_io_fields.nim b/tests/math_fields/t_io_fields.nim index 50500165..5b47c1f2 100644 --- a/tests/math_fields/t_io_fields.nim +++ b/tests/math_fields/t_io_fields.nim @@ -156,4 +156,14 @@ proc main() = check: p == hex + test "Fuzz #1 - incorrect reduction of BigInt": + block: + var a{.noInit.}: Fp[BN254_Snarks] + a.fromBig(BigInt[254].fromHex("0xdd1119d0c5b065898a0848e21c209153f4622f06cb763e7ef00eef28b94780f8")) + + var b{.noInit.}: Fp[BN254_Snarks] + b.fromBig(BigInt[254].fromHex("0x1b7fe00540e9e4e2a8c73208161b2fdd965c84c129af1449ff8cbecd57538bdc")) + + doAssert bool(a == b) + main()