From b99ee137d0cf8e7c5c4250b16832867979d2e22e Mon Sep 17 00:00:00 2001 From: Mamy Ratsimbazafy Date: Tue, 16 Jul 2024 10:19:51 +0200 Subject: [PATCH] fix(arith): bug in vartime inversion when using fused inverse+multiply by factor - found by Guido Vranken --- constantine/math/arithmetic/limbs_exgcd.nim | 4 ++-- tests/math_bigints/t_bigints.nim | 17 +++++++++++++++++ tests/math_fields/t_finite_fields_powinv.nim | 10 ++++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/constantine/math/arithmetic/limbs_exgcd.nim b/constantine/math/arithmetic/limbs_exgcd.nim index fe260641..df69d691 100644 --- a/constantine/math/arithmetic/limbs_exgcd.nim +++ b/constantine/math/arithmetic/limbs_exgcd.nim @@ -854,7 +854,7 @@ func invmod_vartime*( r.setZero() return if a.isOne().bool: - r.setOne() + r = F return const Excess = 2 @@ -888,7 +888,7 @@ func invmod_vartime*( r.setZero() return if a.isOne().bool: - r.setOne() + r = F return const Excess = 2 diff --git a/tests/math_bigints/t_bigints.nim b/tests/math_bigints/t_bigints.nim index afe7f4e5..e5c09a73 100644 --- a/tests/math_bigints/t_bigints.nim +++ b/tests/math_bigints/t_bigints.nim @@ -10,6 +10,7 @@ import # Standard library std/unittest, # Internal + constantine/named/algebras, constantine/math/io/io_bigints, constantine/math/arithmetic, constantine/platforms/abstractions, @@ -658,6 +659,22 @@ proc mainModularInverse() = bool(r == expected) bool(r2 == expected) + test "#433 - CryptoFuzz / Oss-Fuzz bug with unit inversion": + block: + let a = BigInt[255].fromUint(1'u) + let M = BLS12_381.scalarFieldModulus() + let F = Fr[BLS12_381].getR2modP() + + var r = canary(BigInt[255]) + var r2 = canary(BigInt[255]) + + r.invmod(a, F, M) + r2.invmod_vartime(a, F, M) + + check: + bool(r == F) + bool(r2 == F) + mainArith() mainMul() mainMulHigh() diff --git a/tests/math_fields/t_finite_fields_powinv.nim b/tests/math_fields/t_finite_fields_powinv.nim index fd220cc6..00df8a5a 100644 --- a/tests/math_fields/t_finite_fields_powinv.nim +++ b/tests/math_fields/t_finite_fields_powinv.nim @@ -465,4 +465,14 @@ proc main_anti_regression = check: bool(a == expected) + suite "Bug highlighted by 24/7 fuzzing (Guido Vranken's CryptoFuzz / Google-OssFuzz)" & " [" & $WordBitWidth & "-bit words]": + test "#433 - Short-circuit when Montgomery a' = aR (mod p) == 1": + let a = BigInt[255].fromDecimal("12549076656233958353659347336803947287922716146853412054870763148006372261952") + let expected = BigInt[255].fromDecimal("10920338887063814464675503992315976177888879664585288394250266608035967270910") + var aa = Fr[BLS12_381].fromBig(a) + + doAssert bool(aa.mres.isOne()) + aa.inv_vartime() + check: bool(aa.toBig() == expected) + main_anti_regression()