Skip to content

Commit

Permalink
Add support for EIP2098 "short signatures" in the ECDSA library (#2582)
Browse files Browse the repository at this point in the history 
Co-authored-by: Francisco Giordano <[email protected]>
  • Loading branch information
@Amxx @frangio
Amxx and frangio authored Apr 9, 2021
1 parent 057efbf commit 555be63
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 82 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* `ERC20FlashMint`: add an implementation of the ERC3156 extension for flash-minting ERC20 tokens. ([#2543](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2543))
* `SignatureChecker`: add a signature verification library that supports both EOA and ERC1271 compliant contracts as signers. ([#2532](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2532))
* `Multicall`: add abstract contract with `multicall(bytes[] calldata data)` function to bundle multiple calls together ([#2608](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2608))
* `ECDSA`: add support for ERC2098 short-signatures. ([#2582](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2582))

## 4.0.0 (2021-03-23)

Expand Down
36 changes: 24 additions & 12 deletions contracts/utils/cryptography/ECDSA.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,23 +24,35 @@ library ECDSA {
* be too long), and then calling {toEthSignedMessageHash} on it.
*/
function recover(bytes32 hash, bytes memory signature) internal pure returns (address) {
// Check the signature length
if (signature.length != 65) {
revert("ECDSA: invalid signature length");
}

// Divide the signature in r, s and v variables
bytes32 r;
bytes32 s;
uint8 v;

// ecrecover takes the signature parameters, and the only way to get them
// currently is to use assembly.
// solhint-disable-next-line no-inline-assembly
assembly {
r := mload(add(signature, 0x20))
s := mload(add(signature, 0x40))
v := byte(0, mload(add(signature, 0x60)))
// Check the signature length
// - case 65: r,s,v signature (standard)
// - case 64: r,vs signature (cf https://eips.ethereum.org/EIPS/eip-2098)
if (signature.length == 65) {
// ecrecover takes the signature parameters, and the only way to get them
// currently is to use assembly.
// solhint-disable-next-line no-inline-assembly
assembly {
r := mload(add(signature, 0x20))
s := mload(add(signature, 0x40))
v := byte(0, mload(add(signature, 0x60)))
}
} else if (signature.length == 64) {
// ecrecover takes the signature parameters, and the only way to get them
// currently is to use assembly.
// solhint-disable-next-line no-inline-assembly
assembly {
let vs := mload(add(signature, 0x40))
r := mload(add(signature, 0x20))
s := and(vs, 0x7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff)
v := add(shr(255, vs), 27)
}
} else {
revert("ECDSA: invalid signature length");
}

return recover(hash, v, r, s);
Expand Down
150 changes: 80 additions & 70 deletions test/utils/cryptography/ECDSA.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,22 @@ const ECDSAMock = artifacts.require('ECDSAMock');
const TEST_MESSAGE = web3.utils.sha3('OpenZeppelin');
const WRONG_MESSAGE = web3.utils.sha3('Nope');

function to2098Format (signature) {
const long = web3.utils.hexToBytes(signature);
expect(long.length).to.be.equal(65);
const short = long.slice(0, 64);
short[32] |= (long[64] % 27) << 7; // set the first bit of the 32nd byte to the v parity bit
return web3.utils.bytesToHex(short);
}

function from2098Format (signature) {
const short = web3.utils.hexToBytes(signature);
expect(short.length).to.be.equal(64);
short.push((short[32] >> 7) + 27);
short[32] &= (1 << 7) - 1; // zero out the first bit of 1 the 32nd byte
return web3.utils.bytesToHex(short);
}

contract('ECDSA', function (accounts) {
const [ other ] = accounts;

Expand Down Expand Up @@ -36,30 +52,31 @@ contract('ECDSA', function (accounts) {
// eslint-disable-next-line max-len
const signatureWithoutVersion = '0x5d99b6f7f6d1f73d1a26497f2b1c89b24c0993913f86e9a2d02cd69887d9c94f3c880358579d811b21dd1b7fd9bb01c1d81d10e69f0384e675c32b39643be892';

context('with 00 as version value', function () {
it('reverts', async function () {
const version = '00';
const signature = signatureWithoutVersion + version;
await expectRevert(this.ecdsa.recover(TEST_MESSAGE, signature), 'ECDSA: invalid signature \'v\' value');
});
it('reverts with 00 as version value', async function () {
const version = '00';
const signature = signatureWithoutVersion + version;
await expectRevert(this.ecdsa.recover(TEST_MESSAGE, signature), 'ECDSA: invalid signature \'v\' value');
});

it('works with 27 as version value', async function () {
const version = '1b'; // 27 = 1b.
const signature = signatureWithoutVersion + version;
expect(await this.ecdsa.recover(TEST_MESSAGE, signature)).to.equal(signer);
});

context('with 27 as version value', function () {
it('works', async function () {
const version = '1b'; // 27 = 1b.
const signature = signatureWithoutVersion + version;
expect(await this.ecdsa.recover(TEST_MESSAGE, signature)).to.equal(signer);
});
it('reverts with wrong version', async function () {
// The last two hex digits are the signature version.
// The only valid values are 0, 1, 27 and 28.
const version = '02';
const signature = signatureWithoutVersion + version;
await expectRevert(this.ecdsa.recover(TEST_MESSAGE, signature), 'ECDSA: invalid signature \'v\' value');
});

context('with wrong version', function () {
it('reverts', async function () {
// The last two hex digits are the signature version.
// The only valid values are 0, 1, 27 and 28.
const version = '02';
const signature = signatureWithoutVersion + version;
await expectRevert(this.ecdsa.recover(TEST_MESSAGE, signature), 'ECDSA: invalid signature \'v\' value');
});
it('works with short EIP2098 format', async function () {
const version = '1b'; // 27 = 1b.
const signature = signatureWithoutVersion + version;
expect(await this.ecdsa.recover(TEST_MESSAGE, to2098Format(signature))).to.equal(signer);
expect(await this.ecdsa.recover(TEST_MESSAGE, from2098Format(to2098Format(signature)))).to.equal(signer);
});
});

Expand All @@ -68,76 +85,69 @@ contract('ECDSA', function (accounts) {
// eslint-disable-next-line max-len
const signatureWithoutVersion = '0x331fe75a821c982f9127538858900d87d3ec1f9f737338ad67cad133fa48feff48e6fa0c18abc62e42820f05943e47af3e9fbe306ce74d64094bdf1691ee53e0';

context('with 01 as version value', function () {
it('reverts', async function () {
const version = '01';
const signature = signatureWithoutVersion + version;
await expectRevert(this.ecdsa.recover(TEST_MESSAGE, signature), 'ECDSA: invalid signature \'v\' value');
});
it('reverts with 01 as version value', async function () {
const version = '01';
const signature = signatureWithoutVersion + version;
await expectRevert(this.ecdsa.recover(TEST_MESSAGE, signature), 'ECDSA: invalid signature \'v\' value');
});

it('works with 28 as version value', async function () {
const version = '1c'; // 28 = 1c.
const signature = signatureWithoutVersion + version;
expect(await this.ecdsa.recover(TEST_MESSAGE, signature)).to.equal(signer);
});

context('with 28 as version value', function () {
it('works', async function () {
const version = '1c'; // 28 = 1c.
const signature = signatureWithoutVersion + version;
expect(await this.ecdsa.recover(TEST_MESSAGE, signature)).to.equal(signer);
});
it('reverts with wrong version', async function () {
// The last two hex digits are the signature version.
// The only valid values are 0, 1, 27 and 28.
const version = '02';
const signature = signatureWithoutVersion + version;
await expectRevert(this.ecdsa.recover(TEST_MESSAGE, signature), 'ECDSA: invalid signature \'v\' value');
});

context('with wrong version', function () {
it('reverts', async function () {
// The last two hex digits are the signature version.
// The only valid values are 0, 1, 27 and 28.
const version = '02';
const signature = signatureWithoutVersion + version;
await expectRevert(this.ecdsa.recover(TEST_MESSAGE, signature), 'ECDSA: invalid signature \'v\' value');
});
it('works with short EIP2098 format', async function () {
const version = '1c'; // 27 = 1b.
const signature = signatureWithoutVersion + version;
expect(await this.ecdsa.recover(TEST_MESSAGE, to2098Format(signature))).to.equal(signer);
expect(await this.ecdsa.recover(TEST_MESSAGE, from2098Format(to2098Format(signature)))).to.equal(signer);
});
});

context('with high-s value signature', function () {
it('reverts', async function () {
const message = '0xb94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9';
// eslint-disable-next-line max-len
const highSSignature = '0xe742ff452d41413616a5bf43fe15dd88294e983d3d36206c2712f39083d638bde0a0fc89be718fbc1033e1d30d78be1c68081562ed2e97af876f286f3453231d1b';
it('reverts with high-s value signature', async function () {
const message = '0xb94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9';
// eslint-disable-next-line max-len
const highSSignature = '0xe742ff452d41413616a5bf43fe15dd88294e983d3d36206c2712f39083d638bde0a0fc89be718fbc1033e1d30d78be1c68081562ed2e97af876f286f3453231d1b';

await expectRevert(this.ecdsa.recover(message, highSSignature), 'ECDSA: invalid signature \'s\' value');
});
await expectRevert(this.ecdsa.recover(message, highSSignature), 'ECDSA: invalid signature \'s\' value');
});

context('using web3.eth.sign', function () {
context('with correct signature', function () {
it('returns signer address', async function () {
// Create the signature
const signature = fixSignature(await web3.eth.sign(TEST_MESSAGE, other));

// Recover the signer address from the generated message and signature.
expect(await this.ecdsa.recover(
toEthSignedMessageHash(TEST_MESSAGE),
signature,
)).to.equal(other);
});
it('returns signer address with correct signature', async function () {
// Create the signature
const signature = fixSignature(await web3.eth.sign(TEST_MESSAGE, other));

// Recover the signer address from the generated message and signature.
expect(await this.ecdsa.recover(
toEthSignedMessageHash(TEST_MESSAGE),
signature,
)).to.equal(other);
});

context('with wrong message', function () {
it('returns a different address', async function () {
const signature = fixSignature(await web3.eth.sign(TEST_MESSAGE, other));
expect(await this.ecdsa.recover(WRONG_MESSAGE, signature)).to.not.equal(other);
});
it('returns a different address', async function () {
const signature = fixSignature(await web3.eth.sign(TEST_MESSAGE, other));
expect(await this.ecdsa.recover(WRONG_MESSAGE, signature)).to.not.equal(other);
});

context('with invalid signature', function () {
it('reverts', async function () {
// eslint-disable-next-line max-len
const signature = '0x332ce75a821c982f9127538858900d87d3ec1f9f737338ad67cad133fa48feff48e6fa0c18abc62e42820f05943e47af3e9fbe306ce74d64094bdf1691ee53e01c';
await expectRevert(this.ecdsa.recover(TEST_MESSAGE, signature), 'ECDSA: invalid signature');
});
it('reverts with invalid signature', async function () {
// eslint-disable-next-line max-len
const signature = '0x332ce75a821c982f9127538858900d87d3ec1f9f737338ad67cad133fa48feff48e6fa0c18abc62e42820f05943e47af3e9fbe306ce74d64094bdf1691ee53e01c';
await expectRevert(this.ecdsa.recover(TEST_MESSAGE, signature), 'ECDSA: invalid signature');
});
});
});

context('toEthSignedMessage', function () {
it('should prefix hashes correctly', async function () {
it('prefixes hashes correctly', async function () {
expect(await this.ecdsa.toEthSignedMessageHash(TEST_MESSAGE)).to.equal(toEthSignedMessageHash(TEST_MESSAGE));
});
});
Expand Down

0 comments on commit 555be63

Please sign in to comment.