chore: add link to eip-1271

.changeset/plain-times-itch.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': minor
+---
+
+`ECDSA`: Add `parse` and `parseCalldata` to parse bytes signatures of length 65 or 64 (erc-2098) into its v,r,s components.

.github/workflows/formal-verification.yml

@@ -84,3 +84,5 @@ jobs:
         run: pip install -r fv-requirements.txt
       - name: Run Halmos
         run: halmos --match-test '^symbolic|^testSymbolic' -vv
+        env:
+          HALMOS_ALLOW_DOWNLOAD: 1

CHANGELOG.md

@@ -5,6 +5,10 @@
 
 - Update minimum pragma to 0.8.24 in `Votes`, `VotesExtended`, `ERC20Votes`, `Strings`, `ERC1155URIStorage`, `MessageHashUtils`, `ERC721URIStorage`, `ERC721Votes`, `ERC721Wrapper`, `ERC721Burnable`, `ERC721Consecutive`, `ERC721Enumerable`, `ERC721Pausable`, `ERC721Royalty`, `ERC721Wrapper`, `EIP712`, and `ERC7739`. ([#5726](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/5726))
 
+### Deprecation
+
+- `ECDSA` signature malleability protection is partly deprecated. See documentation for more details.
+
 ## 5.4.0 (2025-07-17)
 
 ### Breaking changes

contracts/utils/cryptography/ECDSA.sol

@@ -43,6 +43,10 @@ library ECDSA {
      * this function rejects them by requiring the `s` value to be in the lower
      * half order, and the `v` value to be either 27 or 28.
      *
+     * NOTE: This function only supports 65-byte signatures. ERC-2098 short signatures are rejected. This restriction
+     * is DEPRECATED and will be removed in v6.0. Developers SHOULD NOT use signatures as unique identifiers; use hash
+     * invalidation or nonces for replay protection.
+     *
      * IMPORTANT: `hash` _must_ be the result of a hash operation for the
      * verification to be secure: it is possible to craft signatures that
      * recover to arbitrary addresses for non-hashed data. A safe way to ensure
@@ -107,6 +111,10 @@ library ECDSA {
      * this function rejects them by requiring the `s` value to be in the lower
      * half order, and the `v` value to be either 27 or 28.
      *
+     * NOTE: This function only supports 65-byte signatures. ERC-2098 short signatures are rejected. This restriction
+     * is DEPRECATED and will be removed in v6.0. Developers SHOULD NOT use signatures as unique identifiers; use hash
+     * invalidation or nonces for replay protection.
+     *
      * IMPORTANT: `hash` _must_ be the result of a hash operation for the
      * verification to be secure: it is possible to craft signatures that
      * recover to arbitrary addresses for non-hashed data. A safe way to ensure
@@ -197,6 +205,63 @@ library ECDSA {
         return recovered;
     }
 
+    /**
+     * @dev Parse a signature into its `v`, `r` and `s` components. Supports 65-byte and 64-byte (ERC-2098)
+     * formats. Returns (0,0,0) for invalid signatures. Consider skipping {tryRecover} or {recover} if so.
+     */
+    function parse(bytes memory signature) internal pure returns (uint8 v, bytes32 r, bytes32 s) {
+        assembly ("memory-safe") {
+            // Check the signature length
+            switch mload(signature)
+            // - case 65: r,s,v signature (standard)
+            case 65 {
+                r := mload(add(signature, 0x20))
+                s := mload(add(signature, 0x40))
+                v := byte(0, mload(add(signature, 0x60)))
+            }
+            // - case 64: r,vs signature (cf https://eips.ethereum.org/EIPS/eip-2098)
+            case 64 {
+                let vs := mload(add(signature, 0x40))
+                r := mload(add(signature, 0x20))
+                s := and(vs, shr(1, not(0)))
+                v := add(shr(255, vs), 27)
+            }
+            default {
+                r := 0
+                s := 0
+                v := 0
+            }
+        }
+    }
+
+    /**
+     * @dev Variant of {parse} that takes a signature in calldata
+     */
+    function parseCalldata(bytes calldata signature) internal pure returns (uint8 v, bytes32 r, bytes32 s) {
+        assembly ("memory-safe") {
+            // Check the signature length
+            switch signature.length
+            // - case 65: r,s,v signature (standard)
+            case 65 {
+                r := calldataload(signature.offset)
+                s := calldataload(add(signature.offset, 0x20))
+                v := byte(0, calldataload(add(signature.offset, 0x40)))
+            }
+            // - case 64: r,vs signature (cf https://eips.ethereum.org/EIPS/eip-2098)
+            case 64 {
+                let vs := calldataload(add(signature.offset, 0x20))
+                r := calldataload(signature.offset)
+                s := and(vs, shr(1, not(0)))
+                v := add(shr(255, vs), 27)
+            }
+            default {
+                r := 0
+                s := 0
+                v := 0
+            }
+        }
+    }
+
     /**
      * @dev Optionally reverts with the corresponding custom error according to the `error` argument provided.
      */

docs/modules/ROOT/pages/account-abstraction.adoc

@@ -95,6 +95,6 @@ To process a bundle of `UserOperations`, bundlers call xref:api:account.adoc#Acc
 
 These rules outline the requirements for operations to be processed by the canonical mempool.
 
-Accounts can access their own storage during the validation phase, they might easily violate ERC-7562 storage access rules in indirect ways. For example, most accounts access their public keys from storage when validating a signature, limiting the ability of having accounts that validate operations for other accounts (e.g. via ERC-1271)
+Accounts can access their own storage during the validation phase, they might easily violate ERC-7562 storage access rules in indirect ways. For example, most accounts access their public keys from storage when validating a signature, limiting the ability of having accounts that validate operations for other accounts (e.g. via https://eips.ethereum.org/EIPS/eip-1271[ERC-1271])
 
 TIP: Although any Account that breaks such rules may still be processed by a private bundler, developers should keep in mind the centralization tradeoffs of relying on private infrastructure instead of _permissionless_ execution.

test/utils/Bytes.t.sol

@@ -14,7 +14,7 @@ contract BytesTest is Test {
     }
 
     // INDEX OF
-    function testIndexOf(bytes memory buffer, bytes1 s) public pure {
+    function testSymbolicIndexOf(bytes memory buffer, bytes1 s) public pure {
         uint256 result = Bytes.indexOf(buffer, s);
 
         if (buffer.length == 0) {
@@ -48,7 +48,7 @@ contract BytesTest is Test {
         }
     }
 
-    function testLastIndexOf(bytes memory buffer, bytes1 s) public pure {
+    function testSymbolicLastIndexOf(bytes memory buffer, bytes1 s) public pure {
         uint256 result = Bytes.lastIndexOf(buffer, s);
 
         if (buffer.length == 0) {

test/utils/cryptography/ECDSA.test.js

@@ -19,18 +19,26 @@ describe('ECDSA', function () {
 
   describe('recover with invalid signature', function () {
     it('with short signature', async function () {
-      await expect(this.mock.$recover(TEST_MESSAGE, '0x1234'))
+      const signature = '0x1234';
+
+      await expect(this.mock.$recover(TEST_MESSAGE, signature))
+        .to.be.revertedWithCustomError(this.mock, 'ECDSAInvalidSignatureLength')
+        .withArgs(2);
+
+      await expect(this.mock.$recoverCalldata(TEST_MESSAGE, signature))
         .to.be.revertedWithCustomError(this.mock, 'ECDSAInvalidSignatureLength')
         .withArgs(2);
     });
 
     it('with long signature', async function () {
-      await expect(
-        this.mock.$recover(
-          TEST_MESSAGE,
-          '0x01234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789',
-        ),
-      )
+      const signature =
+        '0x01234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789';
+
+      await expect(this.mock.$recover(TEST_MESSAGE, signature))
+        .to.be.revertedWithCustomError(this.mock, 'ECDSAInvalidSignatureLength')
+        .withArgs(85);
+
+      await expect(this.mock.$recoverCalldata(TEST_MESSAGE, signature))
         .to.be.revertedWithCustomError(this.mock, 'ECDSAInvalidSignatureLength')
         .withArgs(85);
     });
@@ -43,23 +51,29 @@ describe('ECDSA', function () {
         const signature = await this.signer.signMessage(TEST_MESSAGE);
 
         // Recover the signer address from the generated message and signature.
-        expect(await this.mock.$recover(ethers.hashMessage(TEST_MESSAGE), signature)).to.equal(this.signer);
-        expect(await this.mock.$recoverCalldata(ethers.hashMessage(TEST_MESSAGE), signature)).to.equal(this.signer);
+        await expect(this.mock.$recover(ethers.hashMessage(TEST_MESSAGE), signature)).to.eventually.equal(this.signer);
+        await expect(this.mock.$recoverCalldata(ethers.hashMessage(TEST_MESSAGE), signature)).to.eventually.equal(
+          this.signer,
+        );
       });
 
       it('returns signer address with correct signature for arbitrary length message', async function () {
         // Create the signature
         const signature = await this.signer.signMessage(NON_HASH_MESSAGE);
 
         // Recover the signer address from the generated message and signature.
-        expect(await this.mock.$recover(ethers.hashMessage(NON_HASH_MESSAGE), signature)).to.equal(this.signer);
-        expect(await this.mock.$recoverCalldata(ethers.hashMessage(NON_HASH_MESSAGE), signature)).to.equal(this.signer);
+        await expect(this.mock.$recover(ethers.hashMessage(NON_HASH_MESSAGE), signature)).to.eventually.equal(
+          this.signer,
+        );
+        await expect(this.mock.$recoverCalldata(ethers.hashMessage(NON_HASH_MESSAGE), signature)).to.eventually.equal(
+          this.signer,
+        );
       });
 
       it('returns a different address', async function () {
         const signature = await this.signer.signMessage(TEST_MESSAGE);
-        expect(await this.mock.$recover(WRONG_MESSAGE, signature)).to.not.be.equal(this.signer);
-        expect(await this.mock.$recoverCalldata(WRONG_MESSAGE, signature)).to.not.be.equal(this.signer);
+        await expect(this.mock.$recover(WRONG_MESSAGE, signature)).to.eventually.not.equal(this.signer);
+        await expect(this.mock.$recoverCalldata(WRONG_MESSAGE, signature)).to.eventually.not.equal(this.signer);
       });
 
       it('reverts with invalid signature', async function () {
@@ -85,22 +99,24 @@ describe('ECDSA', function () {
       it('works with correct v value', async function () {
         const v = '0x1b'; // 27 = 1b.
         const signature = ethers.concat([signatureWithoutV, v]);
-        expect(await this.mock.$recover(TEST_MESSAGE, signature)).to.equal(signer);
-        expect(await this.mock.$recoverCalldata(TEST_MESSAGE, signature)).to.equal(signer);
+        await expect(this.mock.$recover(TEST_MESSAGE, signature)).to.eventually.equal(signer);
+        await expect(this.mock.$recoverCalldata(TEST_MESSAGE, signature)).to.eventually.equal(signer);
 
         const { r, s, yParityAndS: vs } = ethers.Signature.from(signature);
-        expect(await this.mock.getFunction('$recover(bytes32,uint8,bytes32,bytes32)')(TEST_MESSAGE, v, r, s)).to.equal(
-          signer,
-        );
+        await expect(
+          this.mock.getFunction('$recover(bytes32,uint8,bytes32,bytes32)')(TEST_MESSAGE, v, r, s),
+        ).to.eventually.equal(signer);
 
-        expect(await this.mock.getFunction('$recover(bytes32,bytes32,bytes32)')(TEST_MESSAGE, r, vs)).to.equal(signer);
+        await expect(
+          this.mock.getFunction('$recover(bytes32,bytes32,bytes32)')(TEST_MESSAGE, r, vs),
+        ).to.eventually.equal(signer);
       });
 
       it('rejects incorrect v value', async function () {
         const v = '0x1c'; // 28 = 1c.
         const signature = ethers.concat([signatureWithoutV, v]);
-        expect(await this.mock.$recover(TEST_MESSAGE, signature)).to.not.equal(signer);
-        expect(await this.mock.$recoverCalldata(TEST_MESSAGE, signature)).to.not.equal(signer);
+        await expect(this.mock.$recover(TEST_MESSAGE, signature)).to.eventually.not.equal(signer);
+        await expect(this.mock.$recoverCalldata(TEST_MESSAGE, signature)).to.eventually.not.equal(signer);
 
         const { r, s, yParityAndS: vs } = ethers.Signature.from(signature);
         expect(
@@ -154,29 +170,31 @@ describe('ECDSA', function () {
       it('works with correct v value', async function () {
         const v = '0x1c'; // 28 = 1c.
         const signature = ethers.concat([signatureWithoutV, v]);
-        expect(await this.mock.$recover(TEST_MESSAGE, signature)).to.equal(signer);
-        expect(await this.mock.$recoverCalldata(TEST_MESSAGE, signature)).to.equal(signer);
+        await expect(this.mock.$recover(TEST_MESSAGE, signature)).to.eventually.equal(signer);
+        await expect(this.mock.$recoverCalldata(TEST_MESSAGE, signature)).to.eventually.equal(signer);
 
         const { r, s, yParityAndS: vs } = ethers.Signature.from(signature);
-        expect(await this.mock.getFunction('$recover(bytes32,uint8,bytes32,bytes32)')(TEST_MESSAGE, v, r, s)).to.equal(
-          signer,
-        );
+        await expect(
+          this.mock.getFunction('$recover(bytes32,uint8,bytes32,bytes32)')(TEST_MESSAGE, v, r, s),
+        ).to.eventually.equal(signer);
 
-        expect(await this.mock.getFunction('$recover(bytes32,bytes32,bytes32)')(TEST_MESSAGE, r, vs)).to.equal(signer);
+        await expect(
+          this.mock.getFunction('$recover(bytes32,bytes32,bytes32)')(TEST_MESSAGE, r, vs),
+        ).to.eventually.equal(signer);
       });
 
       it('rejects incorrect v value', async function () {
         const v = '0x1b'; // 27 = 1b.
         const signature = ethers.concat([signatureWithoutV, v]);
-        expect(await this.mock.$recover(TEST_MESSAGE, signature)).to.not.equal(signer);
-        expect(await this.mock.$recoverCalldata(TEST_MESSAGE, signature)).to.not.equal(signer);
+        await expect(this.mock.$recover(TEST_MESSAGE, signature)).to.not.equal(signer);
+        await expect(this.mock.$recoverCalldata(TEST_MESSAGE, signature)).to.not.equal(signer);
 
         const { r, s, yParityAndS: vs } = ethers.Signature.from(signature);
         expect(
           await this.mock.getFunction('$recover(bytes32,uint8,bytes32,bytes32)')(TEST_MESSAGE, v, r, s),
         ).to.not.equal(signer);
 
-        expect(await this.mock.getFunction('$recover(bytes32,bytes32,bytes32)')(TEST_MESSAGE, r, vs)).to.not.equal(
+        await expect(this.mock.getFunction('$recover(bytes32,bytes32,bytes32)')(TEST_MESSAGE, r, vs)).to.not.equal(
           signer,
         );
       });
@@ -236,4 +254,65 @@ describe('ECDSA', function () {
       expect(() => ethers.Signature.from(highSSignature)).to.throw('non-canonical s');
     });
   });
+
+  describe('parse signature', function () {
+    it('65 and 64 bytes signatures', async function () {
+      // Create the signature
+      const signature = await this.signer.signMessage(TEST_MESSAGE).then(ethers.Signature.from);
+
+      await expect(this.mock.$parse(signature.serialized)).to.eventually.deep.equal([
+        signature.v,
+        signature.r,
+        signature.s,
+      ]);
+      await expect(this.mock.$parse(signature.compactSerialized)).to.eventually.deep.equal([
+        signature.v,
+        signature.r,
+        signature.s,
+      ]);
+      await expect(this.mock.$parseCalldata(signature.serialized)).to.eventually.deep.equal([
+        signature.v,
+        signature.r,
+        signature.s,
+      ]);
+      await expect(this.mock.$parseCalldata(signature.compactSerialized)).to.eventually.deep.equal([
+        signature.v,
+        signature.r,
+        signature.s,
+      ]);
+    });
+
+    it('with short signature', async function () {
+      const signature = '0x1234';
+
+      await expect(this.mock.$parse(signature)).to.eventually.deep.equal([0n, ethers.ZeroHash, ethers.ZeroHash]);
+
+      await expect(this.mock.$parseCalldata(signature)).to.eventually.deep.equal([
+        0n,
+        ethers.ZeroHash,
+        ethers.ZeroHash,
+      ]);
+    });
+
+    it('with long signature', async function () {
+      const signature =
+        '0x01234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789';
+
+      await expect(this.mock.$recover(TEST_MESSAGE, signature))
+        .to.be.revertedWithCustomError(this.mock, 'ECDSAInvalidSignatureLength')
+        .withArgs(85);
+
+      await expect(this.mock.$recoverCalldata(TEST_MESSAGE, signature))
+        .to.be.revertedWithCustomError(this.mock, 'ECDSAInvalidSignatureLength')
+        .withArgs(85);
+
+      await expect(this.mock.$parse(signature)).to.eventually.deep.equal([0n, ethers.ZeroHash, ethers.ZeroHash]);
+
+      await expect(this.mock.$parseCalldata(signature)).to.eventually.deep.equal([
+        0n,
+        ethers.ZeroHash,
+        ethers.ZeroHash,
+      ]);
+    });
+  });
 });

test/utils/math/Math.t.sol

@@ -12,7 +12,7 @@ contract MathTest is Test {
     }
 
     // ADD512 & MUL512
-    function testAdd512(uint256 a, uint256 b) public pure {
+    function testSymbolicAdd512(uint256 a, uint256 b) public pure {
         (uint256 high, uint256 low) = Math.add512(a, b);
 
         // test against tryAdd
@@ -60,7 +60,8 @@ contract MathTest is Test {
     }
 
     // CEILDIV
-    function testCeilDiv(uint256 a, uint256 b) public pure {
+    /// @custom:halmos --solver cvc5-int
+    function testSymbolicCeilDiv(uint256 a, uint256 b) public pure {
         vm.assume(b > 0);
 
         uint256 result = Math.ceilDiv(a, b);
@@ -112,17 +113,18 @@ contract MathTest is Test {
         _testInvMod(value, p, true);
     }
 
-    function testInvMod2(uint256 seed) public pure {
+    function testSymbolicInvMod2(uint256 seed) public pure {
         uint256 p = 2; // prime
         _testInvMod(bound(seed, 1, p - 1), p, false);
     }
 
-    function testInvMod17(uint256 seed) public pure {
+    function testSymbolicInvMod17(uint256 seed) public pure {
         uint256 p = 17; // prime
         _testInvMod(bound(seed, 1, p - 1), p, false);
     }
 
-    function testInvMod65537(uint256 seed) public pure {
+    /// @custom:halmos --solver bitwuzla-abs
+    function testSymbolicInvMod65537(uint256 seed) public pure {
         uint256 p = 65537; // prime
         _testInvMod(bound(seed, 1, p - 1), p, false);
     }

test/utils/math/SignedMath.t.sol

@@ -47,8 +47,9 @@ contract SignedMathTest is Test {
         assertEq(result, (a + b) / 2);
     }
 
-    // 2. more complex test, full int256 range
-    function testAverage2(int256 a, int256 b) public pure {
+    // 2. more complex test, full int256 range (solver timeout 0 = no timeout)
+    /// @custom:halmos --solver-timeout-assertion 0
+    function testSymbolicAverage2(int256 a, int256 b) public pure {
         (int256 result, int256 min, int256 max) = (
             SignedMath.average(a, b),
             SignedMath.min(a, b),