From 4f31dcb84743b4c8bb91d605581a9f6ade53ff0c Mon Sep 17 00:00:00 2001
From: Mamy Ratsimbazafy <mamy_github@numforge.co>
Date: Tue, 16 Jul 2024 08:42:17 +0000
Subject: [PATCH] fix(arith): bug in vartime inversion when using fused
 inverse+multiply by factor - found by Guido Vranken (#433)

---
 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()