Skip to content

Commit 1412f92

Browse files
Amxxernestognw
Amxx
and
ernestognw
authored
ECDSA: add parse and tryParse (#5814)
Co-authored-by: ernestognw <[email protected]>
1 parent c3961a4 commit 1412f92

File tree

4 files changed

+183
-30
lines changed

4 files changed

+183
-30
lines changed

‎.changeset/plain-times-itch.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'openzeppelin-solidity': minor
3+
---
4+
5+
`ECDSA`: Add `parse` and `parseCalldata` to parse bytes signatures of length 65 or 64 (erc-2098) into its v,r,s components.

‎CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@
55

66
- 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))
77

8+
### Deprecation
9+
10+
- `ECDSA` signature malleability protection is partly deprecated. See documentation for more details.
11+
812
## 5.4.0 (2025-07-17)
913

1014
### Breaking changes

‎contracts/utils/cryptography/ECDSA.sol

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,10 @@ library ECDSA {
4343
* this function rejects them by requiring the `s` value to be in the lower
4444
* half order, and the `v` value to be either 27 or 28.
4545
*
46+
* NOTE: This function only supports 65-byte signatures. ERC-2098 short signatures are rejected. This restriction
47+
* is DEPRECATED and will be removed in v6.0. Developers SHOULD NOT use signatures as unique identifiers; use hash
48+
* invalidation or nonces for replay protection.
49+
*
4650
* IMPORTANT: `hash` _must_ be the result of a hash operation for the
4751
* verification to be secure: it is possible to craft signatures that
4852
* recover to arbitrary addresses for non-hashed data. A safe way to ensure
@@ -106,6 +110,10 @@ library ECDSA {
106110
* this function rejects them by requiring the `s` value to be in the lower
107111
* half order, and the `v` value to be either 27 or 28.
108112
*
113+
* NOTE: This function only supports 65-byte signatures. ERC-2098 short signatures are rejected. This restriction
114+
* is DEPRECATED and will be removed in v6.0. Developers SHOULD NOT use signatures as unique identifiers; use hash
115+
* invalidation or nonces for replay protection.
116+
*
109117
* IMPORTANT: `hash` _must_ be the result of a hash operation for the
110118
* verification to be secure: it is possible to craft signatures that
111119
* recover to arbitrary addresses for non-hashed data. A safe way to ensure
@@ -196,6 +204,63 @@ library ECDSA {
196204
return recovered;
197205
}
198206

207+
/**
208+
* @dev Parse a signature into its `v`, `r` and `s` components. Supports 65-byte and 64-byte (ERC-2098)
209+
* formats. Returns (0,0,0) for invalid signatures. Consider skipping {tryRecover} or {recover} if so.
210+
*/
211+
function parse(bytes memory signature) internal pure returns (uint8 v, bytes32 r, bytes32 s) {
212+
assembly ("memory-safe") {
213+
// Check the signature length
214+
switch mload(signature)
215+
// - case 65: r,s,v signature (standard)
216+
case 65 {
217+
r := mload(add(signature, 0x20))
218+
s := mload(add(signature, 0x40))
219+
v := byte(0, mload(add(signature, 0x60)))
220+
}
221+
// - case 64: r,vs signature (cf https://eips.ethereum.org/EIPS/eip-2098)
222+
case 64 {
223+
let vs := mload(add(signature, 0x40))
224+
r := mload(add(signature, 0x20))
225+
s := and(vs, shr(1, not(0)))
226+
v := add(shr(255, vs), 27)
227+
}
228+
default {
229+
r := 0
230+
s := 0
231+
v := 0
232+
}
233+
}
234+
}
235+
236+
/**
237+
* @dev Variant of {parse} that takes a signature in calldata
238+
*/
239+
function parseCalldata(bytes calldata signature) internal pure returns (uint8 v, bytes32 r, bytes32 s) {
240+
assembly ("memory-safe") {
241+
// Check the signature length
242+
switch signature.length
243+
// - case 65: r,s,v signature (standard)
244+
case 65 {
245+
r := calldataload(signature.offset)
246+
s := calldataload(add(signature.offset, 0x20))
247+
v := byte(0, calldataload(add(signature.offset, 0x40)))
248+
}
249+
// - case 64: r,vs signature (cf https://eips.ethereum.org/EIPS/eip-2098)
250+
case 64 {
251+
let vs := calldataload(add(signature.offset, 0x20))
252+
r := calldataload(signature.offset)
253+
s := and(vs, shr(1, not(0)))
254+
v := add(shr(255, vs), 27)
255+
}
256+
default {
257+
r := 0
258+
s := 0
259+
v := 0
260+
}
261+
}
262+
}
263+
199264
/**
200265
* @dev Optionally reverts with the corresponding custom error according to the `error` argument provided.
201266
*/

‎test/utils/cryptography/ECDSA.test.js

Lines changed: 109 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -19,18 +19,26 @@ describe('ECDSA', function () {
1919

2020
describe('recover with invalid signature', function () {
2121
it('with short signature', async function () {
22-
await expect(this.mock.$recover(TEST_MESSAGE, '0x1234'))
22+
const signature = '0x1234';
23+
24+
await expect(this.mock.$recover(TEST_MESSAGE, signature))
25+
.to.be.revertedWithCustomError(this.mock, 'ECDSAInvalidSignatureLength')
26+
.withArgs(2);
27+
28+
await expect(this.mock.$recoverCalldata(TEST_MESSAGE, signature))
2329
.to.be.revertedWithCustomError(this.mock, 'ECDSAInvalidSignatureLength')
2430
.withArgs(2);
2531
});
2632

2733
it('with long signature', async function () {
28-
await expect(
29-
this.mock.$recover(
30-
TEST_MESSAGE,
31-
'0x01234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789',
32-
),
33-
)
34+
const signature =
35+
'0x01234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789';
36+
37+
await expect(this.mock.$recover(TEST_MESSAGE, signature))
38+
.to.be.revertedWithCustomError(this.mock, 'ECDSAInvalidSignatureLength')
39+
.withArgs(85);
40+
41+
await expect(this.mock.$recoverCalldata(TEST_MESSAGE, signature))
3442
.to.be.revertedWithCustomError(this.mock, 'ECDSAInvalidSignatureLength')
3543
.withArgs(85);
3644
});
@@ -43,23 +51,29 @@ describe('ECDSA', function () {
4351
const signature = await this.signer.signMessage(TEST_MESSAGE);
4452

4553
// Recover the signer address from the generated message and signature.
46-
expect(await this.mock.$recover(ethers.hashMessage(TEST_MESSAGE), signature)).to.equal(this.signer);
47-
expect(await this.mock.$recoverCalldata(ethers.hashMessage(TEST_MESSAGE), signature)).to.equal(this.signer);
54+
await expect(this.mock.$recover(ethers.hashMessage(TEST_MESSAGE), signature)).to.eventually.equal(this.signer);
55+
await expect(this.mock.$recoverCalldata(ethers.hashMessage(TEST_MESSAGE), signature)).to.eventually.equal(
56+
this.signer,
57+
);
4858
});
4959

5060
it('returns signer address with correct signature for arbitrary length message', async function () {
5161
// Create the signature
5262
const signature = await this.signer.signMessage(NON_HASH_MESSAGE);
5363

5464
// Recover the signer address from the generated message and signature.
55-
expect(await this.mock.$recover(ethers.hashMessage(NON_HASH_MESSAGE), signature)).to.equal(this.signer);
56-
expect(await this.mock.$recoverCalldata(ethers.hashMessage(NON_HASH_MESSAGE), signature)).to.equal(this.signer);
65+
await expect(this.mock.$recover(ethers.hashMessage(NON_HASH_MESSAGE), signature)).to.eventually.equal(
66+
this.signer,
67+
);
68+
await expect(this.mock.$recoverCalldata(ethers.hashMessage(NON_HASH_MESSAGE), signature)).to.eventually.equal(
69+
this.signer,
70+
);
5771
});
5872

5973
it('returns a different address', async function () {
6074
const signature = await this.signer.signMessage(TEST_MESSAGE);
61-
expect(await this.mock.$recover(WRONG_MESSAGE, signature)).to.not.be.equal(this.signer);
62-
expect(await this.mock.$recoverCalldata(WRONG_MESSAGE, signature)).to.not.be.equal(this.signer);
75+
await expect(this.mock.$recover(WRONG_MESSAGE, signature)).to.eventually.not.equal(this.signer);
76+
await expect(this.mock.$recoverCalldata(WRONG_MESSAGE, signature)).to.eventually.not.equal(this.signer);
6377
});
6478

6579
it('reverts with invalid signature', async function () {
@@ -85,22 +99,24 @@ describe('ECDSA', function () {
8599
it('works with correct v value', async function () {
86100
const v = '0x1b'; // 27 = 1b.
87101
const signature = ethers.concat([signatureWithoutV, v]);
88-
expect(await this.mock.$recover(TEST_MESSAGE, signature)).to.equal(signer);
89-
expect(await this.mock.$recoverCalldata(TEST_MESSAGE, signature)).to.equal(signer);
102+
await expect(this.mock.$recover(TEST_MESSAGE, signature)).to.eventually.equal(signer);
103+
await expect(this.mock.$recoverCalldata(TEST_MESSAGE, signature)).to.eventually.equal(signer);
90104

91105
const { r, s, yParityAndS: vs } = ethers.Signature.from(signature);
92-
expect(await this.mock.getFunction('$recover(bytes32,uint8,bytes32,bytes32)')(TEST_MESSAGE, v, r, s)).to.equal(
93-
signer,
94-
);
106+
await expect(
107+
this.mock.getFunction('$recover(bytes32,uint8,bytes32,bytes32)')(TEST_MESSAGE, v, r, s),
108+
).to.eventually.equal(signer);
95109

96-
expect(await this.mock.getFunction('$recover(bytes32,bytes32,bytes32)')(TEST_MESSAGE, r, vs)).to.equal(signer);
110+
await expect(
111+
this.mock.getFunction('$recover(bytes32,bytes32,bytes32)')(TEST_MESSAGE, r, vs),
112+
).to.eventually.equal(signer);
97113
});
98114

99115
it('rejects incorrect v value', async function () {
100116
const v = '0x1c'; // 28 = 1c.
101117
const signature = ethers.concat([signatureWithoutV, v]);
102-
expect(await this.mock.$recover(TEST_MESSAGE, signature)).to.not.equal(signer);
103-
expect(await this.mock.$recoverCalldata(TEST_MESSAGE, signature)).to.not.equal(signer);
118+
await expect(this.mock.$recover(TEST_MESSAGE, signature)).to.eventually.not.equal(signer);
119+
await expect(this.mock.$recoverCalldata(TEST_MESSAGE, signature)).to.eventually.not.equal(signer);
104120

105121
const { r, s, yParityAndS: vs } = ethers.Signature.from(signature);
106122
expect(
@@ -154,29 +170,31 @@ describe('ECDSA', function () {
154170
it('works with correct v value', async function () {
155171
const v = '0x1c'; // 28 = 1c.
156172
const signature = ethers.concat([signatureWithoutV, v]);
157-
expect(await this.mock.$recover(TEST_MESSAGE, signature)).to.equal(signer);
158-
expect(await this.mock.$recoverCalldata(TEST_MESSAGE, signature)).to.equal(signer);
173+
await expect(this.mock.$recover(TEST_MESSAGE, signature)).to.eventually.equal(signer);
174+
await expect(this.mock.$recoverCalldata(TEST_MESSAGE, signature)).to.eventually.equal(signer);
159175

160176
const { r, s, yParityAndS: vs } = ethers.Signature.from(signature);
161-
expect(await this.mock.getFunction('$recover(bytes32,uint8,bytes32,bytes32)')(TEST_MESSAGE, v, r, s)).to.equal(
162-
signer,
163-
);
177+
await expect(
178+
this.mock.getFunction('$recover(bytes32,uint8,bytes32,bytes32)')(TEST_MESSAGE, v, r, s),
179+
).to.eventually.equal(signer);
164180

165-
expect(await this.mock.getFunction('$recover(bytes32,bytes32,bytes32)')(TEST_MESSAGE, r, vs)).to.equal(signer);
181+
await expect(
182+
this.mock.getFunction('$recover(bytes32,bytes32,bytes32)')(TEST_MESSAGE, r, vs),
183+
).to.eventually.equal(signer);
166184
});
167185

168186
it('rejects incorrect v value', async function () {
169187
const v = '0x1b'; // 27 = 1b.
170188
const signature = ethers.concat([signatureWithoutV, v]);
171-
expect(await this.mock.$recover(TEST_MESSAGE, signature)).to.not.equal(signer);
172-
expect(await this.mock.$recoverCalldata(TEST_MESSAGE, signature)).to.not.equal(signer);
189+
await expect(this.mock.$recover(TEST_MESSAGE, signature)).to.not.equal(signer);
190+
await expect(this.mock.$recoverCalldata(TEST_MESSAGE, signature)).to.not.equal(signer);
173191

174192
const { r, s, yParityAndS: vs } = ethers.Signature.from(signature);
175193
expect(
176194
await this.mock.getFunction('$recover(bytes32,uint8,bytes32,bytes32)')(TEST_MESSAGE, v, r, s),
177195
).to.not.equal(signer);
178196

179-
expect(await this.mock.getFunction('$recover(bytes32,bytes32,bytes32)')(TEST_MESSAGE, r, vs)).to.not.equal(
197+
await expect(this.mock.getFunction('$recover(bytes32,bytes32,bytes32)')(TEST_MESSAGE, r, vs)).to.not.equal(
180198
signer,
181199
);
182200
});
@@ -236,4 +254,65 @@ describe('ECDSA', function () {
236254
expect(() => ethers.Signature.from(highSSignature)).to.throw('non-canonical s');
237255
});
238256
});
257+
258+
describe('parse signature', function () {
259+
it('65 and 64 bytes signatures', async function () {
260+
// Create the signature
261+
const signature = await this.signer.signMessage(TEST_MESSAGE).then(ethers.Signature.from);
262+
263+
await expect(this.mock.$parse(signature.serialized)).to.eventually.deep.equal([
264+
signature.v,
265+
signature.r,
266+
signature.s,
267+
]);
268+
await expect(this.mock.$parse(signature.compactSerialized)).to.eventually.deep.equal([
269+
signature.v,
270+
signature.r,
271+
signature.s,
272+
]);
273+
await expect(this.mock.$parseCalldata(signature.serialized)).to.eventually.deep.equal([
274+
signature.v,
275+
signature.r,
276+
signature.s,
277+
]);
278+
await expect(this.mock.$parseCalldata(signature.compactSerialized)).to.eventually.deep.equal([
279+
signature.v,
280+
signature.r,
281+
signature.s,
282+
]);
283+
});
284+
285+
it('with short signature', async function () {
286+
const signature = '0x1234';
287+
288+
await expect(this.mock.$parse(signature)).to.eventually.deep.equal([0n, ethers.ZeroHash, ethers.ZeroHash]);
289+
290+
await expect(this.mock.$parseCalldata(signature)).to.eventually.deep.equal([
291+
0n,
292+
ethers.ZeroHash,
293+
ethers.ZeroHash,
294+
]);
295+
});
296+
297+
it('with long signature', async function () {
298+
const signature =
299+
'0x01234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789';
300+
301+
await expect(this.mock.$recover(TEST_MESSAGE, signature))
302+
.to.be.revertedWithCustomError(this.mock, 'ECDSAInvalidSignatureLength')
303+
.withArgs(85);
304+
305+
await expect(this.mock.$recoverCalldata(TEST_MESSAGE, signature))
306+
.to.be.revertedWithCustomError(this.mock, 'ECDSAInvalidSignatureLength')
307+
.withArgs(85);
308+
309+
await expect(this.mock.$parse(signature)).to.eventually.deep.equal([0n, ethers.ZeroHash, ethers.ZeroHash]);
310+
311+
await expect(this.mock.$parseCalldata(signature)).to.eventually.deep.equal([
312+
0n,
313+
ethers.ZeroHash,
314+
ethers.ZeroHash,
315+
]);
316+
});
317+
});
239318
});

0 commit comments

Comments
 (0)