From 4a4803bb62ca06c8b6ee552ca24cf0002ddae624 Mon Sep 17 00:00:00 2001
From: qd-qd <qdqdqdqdqd@protonmail.com>
Date: Fri, 15 Dec 2023 16:26:40 +0100
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20fix=20inlined=20`ec=5FDbl`=20in?=
 =?UTF-8?q?=20shamir-strauss?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

In the FCL repository, @nlordell pointed out the mulmuladd function had a
bug revealed by the edge cases committed by @rdubois-crypto.
This PR fixes that bug like @nlordell did and add the relevant edge cases.
Link of the initial PR: https://github.com/rdubois-crypto/FreshCryptoLib/pull/5

Co-authored-by: Nicholas Rodrigues Lordello <nick@safe.global>
Co-authored-by: rdubois-crypto <renaud.dubois.fr@gmail.com>
---
 src/ECDSA256r1.sol         | 14 ++++++++------
 test/unit/ecdsa256r1.t.sol | 20 ++++++++++++++++++++
 2 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/src/ECDSA256r1.sol b/src/ECDSA256r1.sol
index 75ede13..40a0b38 100644
--- a/src/ECDSA256r1.sol
+++ b/src/ECDSA256r1.sol
@@ -131,15 +131,17 @@ library ECDSA256r1 {
                                 T3 := mulmod(X, T2, p)
                                 // W=UV
                                 T1 := mulmod(T1, T2, p)
-                                y2 := addmod(X, zz, p)
-                                let TT1 := addmod(X, sub(p, zz), p)
-                                // X-ZZ)(X+ZZ)
-                                y2 := mulmod(y2, TT1, p)
-                                // M
+
+                                //(X-ZZ)(X+ZZ) -- xPlusZZ/xMinusZZ needed for stack management
+                                let xPlusZZ := addmod(X, sub(p, zz), p)
+                                let xMinusZZ := addmod(X, zz, p)
+                                y2 := mulmod(xMinusZZ, xPlusZZ, p)
+
+                                //M=3*(X-ZZ)(X+ZZ)
                                 T4 := mulmod(3, y2, p)
 
                                 // zzz3=W*zzz1
-                                zzz := mulmod(TT1, zzz, p)
+                                zzz := mulmod(T1, zzz, p)
                                 // zz3=V*ZZ1, V free
                                 zz := mulmod(T2, zz, p)
 
diff --git a/test/unit/ecdsa256r1.t.sol b/test/unit/ecdsa256r1.t.sol
index cc3c080..623d9db 100644
--- a/test/unit/ecdsa256r1.t.sol
+++ b/test/unit/ecdsa256r1.t.sol
@@ -139,6 +139,26 @@ contract Ecdsa256r1Test is PRBTest {
         uint256 _4P_res3 = implementation.mulmuladd(nQ[0], nQ[1], 2, 1);
 
         assertEq(_4P_res1, _4P_res3);
+
+        // edge case from nlordell
+        (uint256 niordellX, uint256 niordellY, uint256 niordellU, uint256 niordellV) = (
+            0xe2534a3532d08fbba02dde659ee62bd0031fe2db785596ef509302446b030852, //x
+            0x1f0ea8a4b39cc339e62011a02579d289b103693d0cf11ffaa3bd3dc0e7b12739, //y
+            0xd13800358b760290af0671ee67368e9702a7145d1b9a0024b0b61ffe7bce9214, //u
+            0x344e000d62dd80a42bc19c7b99cda3a5c0a9c51746e680092c2d87ff9ef3af6f //v
+        );
+        assertEq(
+            0xcfcfa95b195904fd97b548d9e3cd2e023e06b4f10a87c645c7d4f74a0e206bad,
+            implementation.mulmuladd(niordellX, niordellY, niordellU, niordellV)
+        );
+
+        // edge case for Shamir
+        (uint256 shamirK, uint256 shamirX,) = (
+            0xffffffff00000000ffffffffffffffffbce6faada7179e84f3b9cac2fc63254f, //k
+            0x7CF27B188D034F7E8A52380304B51AC3C08969E277F21B35A60B48FC47669978, // x
+            0xF888AAEE24712FC0D6C26539608BCF244582521AC3167DD661FB4862DD878C2E // y
+        );
+        assertEq(shamirX, implementation.mulmuladd(0, 0, shamirK, 0));
     }
 
     function test_MulMullAddMultipleBy0Fail_ReportSkip(uint256 q0, uint256 q1) public {