Skip to content

Commit 0b388b0

Browse files
ernestognwernestognw
committed
Merge branch 'master' into typo-fixes
2 parents 7dcf761 + 3790c59 commit 0b388b0

File tree

8 files changed

+197
-39
lines changed

8 files changed

+197
-39
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.

‎.github/workflows/formal-verification.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,3 +84,5 @@ jobs:
8484
run: pip install -r fv-requirements.txt
8585
- name: Run Halmos
8686
run: halmos --match-test '^symbolic|^testSymbolic' -vv
87+
env:
88+
HALMOS_ALLOW_DOWNLOAD: 1

‎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
@@ -107,6 +111,10 @@ library ECDSA {
107111
* this function rejects them by requiring the `s` value to be in the lower
108112
* half order, and the `v` value to be either 27 or 28.
109113
*
114+
* NOTE: This function only supports 65-byte signatures. ERC-2098 short signatures are rejected. This restriction
115+
* is DEPRECATED and will be removed in v6.0. Developers SHOULD NOT use signatures as unique identifiers; use hash
116+
* invalidation or nonces for replay protection.
117+
*
110118
* IMPORTANT: `hash` _must_ be the result of a hash operation for the
111119
* verification to be secure: it is possible to craft signatures that
112120
* recover to arbitrary addresses for non-hashed data. A safe way to ensure
@@ -197,6 +205,63 @@ library ECDSA {
197205
return recovered;
198206
}
199207

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

‎test/utils/Bytes.t.sol

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ contract BytesTest is Test {
1414
}
1515

1616
// INDEX OF
17-
function testIndexOf(bytes memory buffer, bytes1 s) public pure {
17+
function testSymbolicIndexOf(bytes memory buffer, bytes1 s) public pure {
1818
uint256 result = Bytes.indexOf(buffer, s);
1919

2020
if (buffer.length == 0) {
@@ -48,7 +48,7 @@ contract BytesTest is Test {
4848
}
4949
}
5050

51-
function testLastIndexOf(bytes memory buffer, bytes1 s) public pure {
51+
function testSymbolicLastIndexOf(bytes memory buffer, bytes1 s) public pure {
5252
uint256 result = Bytes.lastIndexOf(buffer, s);
5353

5454
if (buffer.length == 0) {

‎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
});

‎test/utils/math/Math.t.sol

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ contract MathTest is Test {
1212
}
1313

1414
// ADD512 & MUL512
15-
function testAdd512(uint256 a, uint256 b) public pure {
15+
function testSymbolicAdd512(uint256 a, uint256 b) public pure {
1616
(uint256 high, uint256 low) = Math.add512(a, b);
1717

1818
// test against tryAdd
@@ -60,7 +60,8 @@ contract MathTest is Test {
6060
}
6161

6262
// CEILDIV
63-
function testCeilDiv(uint256 a, uint256 b) public pure {
63+
/// @custom:halmos --solver cvc5-int
64+
function testSymbolicCeilDiv(uint256 a, uint256 b) public pure {
6465
vm.assume(b > 0);
6566

6667
uint256 result = Math.ceilDiv(a, b);
@@ -112,17 +113,18 @@ contract MathTest is Test {
112113
_testInvMod(value, p, true);
113114
}
114115

115-
function testInvMod2(uint256 seed) public pure {
116+
function testSymbolicInvMod2(uint256 seed) public pure {
116117
uint256 p = 2; // prime
117118
_testInvMod(bound(seed, 1, p - 1), p, false);
118119
}
119120

120-
function testInvMod17(uint256 seed) public pure {
121+
function testSymbolicInvMod17(uint256 seed) public pure {
121122
uint256 p = 17; // prime
122123
_testInvMod(bound(seed, 1, p - 1), p, false);
123124
}
124125

125-
function testInvMod65537(uint256 seed) public pure {
126+
/// @custom:halmos --solver bitwuzla-abs
127+
function testSymbolicInvMod65537(uint256 seed) public pure {
126128
uint256 p = 65537; // prime
127129
_testInvMod(bound(seed, 1, p - 1), p, false);
128130
}

‎test/utils/math/SignedMath.t.sol

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,9 @@ contract SignedMathTest is Test {
4747
assertEq(result, (a + b) / 2);
4848
}
4949

50-
// 2. more complex test, full int256 range
51-
function testAverage2(int256 a, int256 b) public pure {
50+
// 2. more complex test, full int256 range (solver timeout 0 = no timeout)
51+
/// @custom:halmos --solver-timeout-assertion 0
52+
function testSymbolicAverage2(int256 a, int256 b) public pure {
5253
(int256 result, int256 min, int256 max) = (
5354
SignedMath.average(a, b),
5455
SignedMath.min(a, b),

0 commit comments

Comments
 (0)