Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for EIP2098 "short signatures" in the ECDSA library #2582

Merged
merged 15 commits into from Apr 9, 2021
Merged
1 change: 1 addition & 0 deletions CHANGELOG.md
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
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
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