From 19e1d63341315c7655a584f01b675b5855f59505 Mon Sep 17 00:00:00 2001
From: Mikhail <16622558+mmv08@users.noreply.github.com>
Date: Thu, 9 Jan 2025 18:23:43 +0100
Subject: [PATCH] Enhance Safe.sol with ECDSA malleability warning (#877)

Added a comment in the Safe contract to clarify that the `s` value of
ECDSA signatures is not enforced to be in the lower half of the curve.
This note explains the implications of ECDSA malleability and reassures
that existing mechanisms are in place to prevent duplicate signatures
and replay attacks. No functional changes were made to the contract
logic.
---
 contracts/Safe.sol | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/contracts/Safe.sol b/contracts/Safe.sol
index a05499a33..975dc836f 100644
--- a/contracts/Safe.sol
+++ b/contracts/Safe.sol
@@ -291,6 +291,10 @@ contract Safe is
         address currentOwner;
         uint256 v; // Implicit conversion from uint8 to uint256 will be done for v received from signatureSplit(...).
         bytes32 r;
+        // NOTE: We do not enforce the `s` to be from the lower half of the curve
+        // This essentially means that for every signature, there's another valid signature (known as ECDSA malleability)
+        // Since we have other mechanisms to prevent duplicated signatures (ordered owners array) and replay protection (nonce),
+        // we can safely ignore this malleability.
         bytes32 s;
         uint256 i;
         for (i = 0; i < requiredSignatures; i++) {