ECDSA: add parse and tryParse (#5814)

Co-authored-by: ernestognw <ernestognw@gmail.com>

.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.

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
@@ -106,6 +110,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
@@ -196,6 +204,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.
      */

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,8 +51,10 @@ 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 () {
@@ -52,14 +62,18 @@ describe('ECDSA', function () {
         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,
+      ]);
+    });
+  });
 });