From dc168f3bf6608cdbc3f51f2dda4560815afd482c Mon Sep 17 00:00:00 2001 From: Thomas Bocek Date: Wed, 23 Jan 2019 18:06:19 +0100 Subject: [PATCH 01/10] Transaction Malleability: If you allow for both values 0/1 and 27/28, you allow two different signatures both resulting in a same valid recovery. (r,s,0/1) and (r,s,27/28) would both be valid, recover the same public key and sign the same data. Furthermore, given (r,s,0/1), (r,s,27/28) can be constructed by anyone. --- contracts/cryptography/ECDSA.sol | 5 ----- test/cryptography/ECDSA.test.js | 14 ++++++++------ test/helpers/sign.js | 10 ++++++++++ 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/contracts/cryptography/ECDSA.sol b/contracts/cryptography/ECDSA.sol index 39b6ad476fe..6aefc57b4a2 100644 --- a/contracts/cryptography/ECDSA.sol +++ b/contracts/cryptography/ECDSA.sol @@ -33,11 +33,6 @@ library ECDSA { v := byte(0, mload(add(signature, 0x60))) } - // Version of signature should be 27 or 28, but 0 and 1 are also possible versions - if (v < 27) { - v += 27; - } - // If the version is correct return the signer address if (v != 27 && v != 28) { return (address(0)); diff --git a/test/cryptography/ECDSA.test.js b/test/cryptography/ECDSA.test.js index c37f405bdcf..5cf4c6a3d0a 100644 --- a/test/cryptography/ECDSA.test.js +++ b/test/cryptography/ECDSA.test.js @@ -1,5 +1,5 @@ const { shouldFail } = require('openzeppelin-test-helpers'); -const { signMessage, toEthSignedMessageHash } = require('../helpers/sign'); +const { signMessage, toEthSignedMessageHash, fixSignature } = require('../helpers/sign'); const ECDSAMock = artifacts.require('ECDSAMock'); @@ -18,11 +18,12 @@ contract('ECDSA', function ([_, anyone]) { // eslint-disable-next-line max-len const signatureWithoutVersion = '0x5d99b6f7f6d1f73d1a26497f2b1c89b24c0993913f86e9a2d02cd69887d9c94f3c880358579d811b21dd1b7fd9bb01c1d81d10e69f0384e675c32b39643be892'; - context('with 00 as version value', function () { + context('with 00 as version value (wrong)', function () { it('works', async function () { const version = '00'; const signature = signatureWithoutVersion + version; - (await this.ecdsa.recover(TEST_MESSAGE, signature)).should.equal(signer); + (await this.ecdsa.recover(TEST_MESSAGE, signature)).should.equal( + '0x0000000000000000000000000000000000000000'); }); }); @@ -51,11 +52,12 @@ contract('ECDSA', function ([_, anyone]) { // eslint-disable-next-line max-len const signatureWithoutVersion = '0x331fe75a821c982f9127538858900d87d3ec1f9f737338ad67cad133fa48feff48e6fa0c18abc62e42820f05943e47af3e9fbe306ce74d64094bdf1691ee53e0'; - context('with 01 as version value', function () { + context('with 01 as version value (wrong)', function () { it('works', async function () { const version = '01'; const signature = signatureWithoutVersion + version; - (await this.ecdsa.recover(TEST_MESSAGE, signature)).should.equal(signer); + (await this.ecdsa.recover(TEST_MESSAGE, signature)).should.equal( + '0x0000000000000000000000000000000000000000'); }); }); @@ -83,7 +85,7 @@ contract('ECDSA', function ([_, anyone]) { context('with correct signature', function () { it('returns signer address', async function () { // Create the signature - const signature = await signMessage(anyone, TEST_MESSAGE); + const signature = fixSignature(await signMessage(anyone, TEST_MESSAGE)); // Recover the signer address from the generated message and signature. (await this.ecdsa.recover( diff --git a/test/helpers/sign.js b/test/helpers/sign.js index 1c60b648495..c327ae800ad 100644 --- a/test/helpers/sign.js +++ b/test/helpers/sign.js @@ -9,6 +9,15 @@ function toEthSignedMessageHash (messageHex) { return web3.utils.sha3(Buffer.concat([prefix, messageBuffer])); } +function fixSignature (signature) { + //in geth its always 27/28, in ganache its 0/1. Change to 27/28 to prevent + //signature malleability + //https://github.com/ethereum/go-ethereum/blob/master/internal/ethapi/api.go#L447 + const v = parseInt(signature.slice(130, 132), 16) + 27; + const vHex = v.toString(16); + return signature.slice(0, 130) + vHex; +} + // signs message in node (ganache auto-applies "Ethereum Signed Message" prefix) const signMessage = (signer, messageHex = '0x') => { return web3.eth.sign(messageHex, signer); @@ -50,5 +59,6 @@ const getSignFor = (contract, signer) => (redeemer, methodName, methodArgs = []) module.exports = { signMessage, toEthSignedMessageHash, + fixSignature, getSignFor, }; From 19d223623a2134eab44a13f16a092fa6b2954413 Mon Sep 17 00:00:00 2001 From: Thomas Bocek Date: Mon, 4 Feb 2019 11:17:12 +0100 Subject: [PATCH 02/10] Transaction Malleability: EIP-2 still allows signature malleabality for ecrecover(), remove this possibility and force the signature to be unique. --- contracts/cryptography/ECDSA.sol | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/contracts/cryptography/ECDSA.sol b/contracts/cryptography/ECDSA.sol index 6aefc57b4a2..920d5bc6ca7 100644 --- a/contracts/cryptography/ECDSA.sol +++ b/contracts/cryptography/ECDSA.sol @@ -33,6 +33,15 @@ library ECDSA { v := byte(0, mload(add(signature, 0x60))) } + //EIP-2 still allows signature malleabality for ecrecover(), remove this possibility and make + //the signature unique. Most signatures from current libraries are generate a unique signature with + //an s-value in the lower half order. If not, calculate a new s-value with + //0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141 - s1 and flip v from 27 to 28 or + //vice versa. It could also happen that a signature uses 0/1 instead 27/28 for v. In that case add 27. + if(uint256(s) > uint256(0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0)) { + return address(0); + } + // If the version is correct return the signer address if (v != 27 && v != 28) { return (address(0)); From 9601e522a54681562ced7c35bfdc963bba1c2b32 Mon Sep 17 00:00:00 2001 From: tbocek Date: Mon, 4 Mar 2019 19:36:13 +0100 Subject: [PATCH 03/10] Added a reference to appendix F to the yellow paper and improved comment. --- contracts/cryptography/ECDSA.sol | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/contracts/cryptography/ECDSA.sol b/contracts/cryptography/ECDSA.sol index 920d5bc6ca7..8ae087564f7 100644 --- a/contracts/cryptography/ECDSA.sol +++ b/contracts/cryptography/ECDSA.sol @@ -33,11 +33,15 @@ library ECDSA { v := byte(0, mload(add(signature, 0x60))) } - //EIP-2 still allows signature malleabality for ecrecover(), remove this possibility and make - //the signature unique. Most signatures from current libraries are generate a unique signature with - //an s-value in the lower half order. If not, calculate a new s-value with - //0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141 - s1 and flip v from 27 to 28 or - //vice versa. It could also happen that a signature uses 0/1 instead 27/28 for v. In that case add 27. + //EIP-2 still allows signature malleability for ecrecover(). Remove this possibility and make the signature + //unique. Appendix F in the Ethereum Yellow paper (https://ethereum.github.io/yellowpaper/paper.pdf), defines + //the valid range for s in (281): 0 < s < secp256k1n ÷ 2 + 1. Most signatures from current libraries generate + //a unique signature with an s-value in the lower half order. + // + //If your library generates malleable signatures, such as s-values in the upper range, calculate a new s-value + //with 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141 - s1 and flip v from 27 to 28 or + //vice versa. If your library also generates signatures with 0/1 for v instead 27/28, add 27 to v to accept + //these malleable signatures as well. if(uint256(s) > uint256(0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0)) { return address(0); } From 8edafa9d854df1cc4800fef262e301293128347d Mon Sep 17 00:00:00 2001 From: tbocek Date: Mon, 4 Mar 2019 20:02:29 +0100 Subject: [PATCH 04/10] better test description for testing the version 0, which returns a zero address --- test/cryptography/ECDSA.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/cryptography/ECDSA.test.js b/test/cryptography/ECDSA.test.js index 5cf4c6a3d0a..9e22ae93ef3 100644 --- a/test/cryptography/ECDSA.test.js +++ b/test/cryptography/ECDSA.test.js @@ -18,8 +18,8 @@ contract('ECDSA', function ([_, anyone]) { // eslint-disable-next-line max-len const signatureWithoutVersion = '0x5d99b6f7f6d1f73d1a26497f2b1c89b24c0993913f86e9a2d02cd69887d9c94f3c880358579d811b21dd1b7fd9bb01c1d81d10e69f0384e675c32b39643be892'; - context('with 00 as version value (wrong)', function () { - it('works', async function () { + context('with 00 as version value', function () { + it('returns 0', async function () { const version = '00'; const signature = signatureWithoutVersion + version; (await this.ecdsa.recover(TEST_MESSAGE, signature)).should.equal( From 4abfaf18e24324135bc572a69ab8455c06c5289f Mon Sep 17 00:00:00 2001 From: tbocek Date: Mon, 4 Mar 2019 20:23:04 +0100 Subject: [PATCH 05/10] Check that the conversion from 0/1 to 27/28 only happens if its 0/1 --- test/helpers/sign.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/test/helpers/sign.js b/test/helpers/sign.js index c327ae800ad..6398173e36b 100644 --- a/test/helpers/sign.js +++ b/test/helpers/sign.js @@ -10,10 +10,13 @@ function toEthSignedMessageHash (messageHex) { } function fixSignature (signature) { - //in geth its always 27/28, in ganache its 0/1. Change to 27/28 to prevent - //signature malleability - //https://github.com/ethereum/go-ethereum/blob/master/internal/ethapi/api.go#L447 - const v = parseInt(signature.slice(130, 132), 16) + 27; + // in geth its always 27/28, in ganache its 0/1. Change to 27/28 to prevent + // signature malleability if version is 0/1 + // see https://github.com/ethereum/go-ethereum/blob/v1.8.23/internal/ethapi/api.go#L465 + let v = parseInt(signature.slice(130, 132), 16); + if (v < 27) { + v += 27; + } const vHex = v.toString(16); return signature.slice(0, 130) + vHex; } From 27e68a4e8a75e284ee87dd53906c07ea84aa3510 Mon Sep 17 00:00:00 2001 From: tbocek Date: Mon, 4 Mar 2019 20:24:28 +0100 Subject: [PATCH 06/10] improved formatting --- contracts/cryptography/ECDSA.sol | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/contracts/cryptography/ECDSA.sol b/contracts/cryptography/ECDSA.sol index 8ae087564f7..c47630bbeb4 100644 --- a/contracts/cryptography/ECDSA.sol +++ b/contracts/cryptography/ECDSA.sol @@ -33,15 +33,15 @@ library ECDSA { v := byte(0, mload(add(signature, 0x60))) } - //EIP-2 still allows signature malleability for ecrecover(). Remove this possibility and make the signature - //unique. Appendix F in the Ethereum Yellow paper (https://ethereum.github.io/yellowpaper/paper.pdf), defines - //the valid range for s in (281): 0 < s < secp256k1n ÷ 2 + 1. Most signatures from current libraries generate - //a unique signature with an s-value in the lower half order. + // EIP-2 still allows signature malleability for ecrecover(). Remove this possibility and make the signature + // unique. Appendix F in the Ethereum Yellow paper (https://ethereum.github.io/yellowpaper/paper.pdf), defines + // the valid range for s in (281): 0 < s < secp256k1n ÷ 2 + 1. Most signatures from current libraries generate + // a unique signature with an s-value in the lower half order. // - //If your library generates malleable signatures, such as s-values in the upper range, calculate a new s-value - //with 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141 - s1 and flip v from 27 to 28 or - //vice versa. If your library also generates signatures with 0/1 for v instead 27/28, add 27 to v to accept - //these malleable signatures as well. + // If your library generates malleable signatures, such as s-values in the upper range, calculate a new s-value + // with 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141 - s1 and flip v from 27 to 28 or + // vice versa. If your library also generates signatures with 0/1 for v instead 27/28, add 27 to v to accept + // these malleable signatures as well. if(uint256(s) > uint256(0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0)) { return address(0); } From 69b1b3c786539d1cbbf62a23310d0f7577ed121d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Wed, 6 Mar 2019 15:25:41 -0300 Subject: [PATCH 07/10] Refactor ECDSA code a bit. --- contracts/cryptography/ECDSA.sol | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/contracts/cryptography/ECDSA.sol b/contracts/cryptography/ECDSA.sol index c47630bbeb4..26b81114e3b 100644 --- a/contracts/cryptography/ECDSA.sol +++ b/contracts/cryptography/ECDSA.sol @@ -14,16 +14,16 @@ library ECDSA { * @param signature bytes signature, the signature is generated using web3.eth.sign() */ function recover(bytes32 hash, bytes memory signature) internal pure returns (address) { - bytes32 r; - bytes32 s; - uint8 v; - // Check the signature length if (signature.length != 65) { return (address(0)); } // 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 @@ -35,23 +35,23 @@ library ECDSA { // EIP-2 still allows signature malleability for ecrecover(). Remove this possibility and make the signature // unique. Appendix F in the Ethereum Yellow paper (https://ethereum.github.io/yellowpaper/paper.pdf), defines - // the valid range for s in (281): 0 < s < secp256k1n ÷ 2 + 1. Most signatures from current libraries generate - // a unique signature with an s-value in the lower half order. + // the valid range for s in (281): 0 < s < secp256k1n ÷ 2 + 1, and for v in (282): v ∈ {27, 28}. Most + // signatures from current libraries generate a unique signature with an s-value in the lower half order. // // If your library generates malleable signatures, such as s-values in the upper range, calculate a new s-value // with 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141 - s1 and flip v from 27 to 28 or // vice versa. If your library also generates signatures with 0/1 for v instead 27/28, add 27 to v to accept // these malleable signatures as well. - if(uint256(s) > uint256(0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0)) { + if (uint256(s) > uint256(0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0)) { return address(0); } - // If the version is correct return the signer address if (v != 27 && v != 28) { - return (address(0)); - } else { - return ecrecover(hash, v, r, s); + return address(0); } + + // If the signature is valid (and not malleable), return the signer address + return ecrecover(hash, v, r, s); } /** From e54d3a67fe912b5b043273321f3f69c530cc0774 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Wed, 6 Mar 2019 15:45:44 -0300 Subject: [PATCH 08/10] Refactor ECDSA tests a bit. --- test/cryptography/ECDSA.test.js | 45 +++++++++++++++------------------ test/helpers/sign.js | 4 +-- 2 files changed, 23 insertions(+), 26 deletions(-) diff --git a/test/cryptography/ECDSA.test.js b/test/cryptography/ECDSA.test.js index 9e22ae93ef3..76bb9c43bab 100644 --- a/test/cryptography/ECDSA.test.js +++ b/test/cryptography/ECDSA.test.js @@ -1,5 +1,6 @@ -const { shouldFail } = require('openzeppelin-test-helpers'); -const { signMessage, toEthSignedMessageHash, fixSignature } = require('../helpers/sign'); +const { constants, shouldFail } = require('openzeppelin-test-helpers'); +const { ZERO_ADDRESS } = constants; +const { toEthSignedMessageHash, fixSignature } = require('../helpers/sign'); const ECDSAMock = artifacts.require('ECDSAMock'); @@ -22,8 +23,7 @@ contract('ECDSA', function ([_, anyone]) { it('returns 0', async function () { const version = '00'; const signature = signatureWithoutVersion + version; - (await this.ecdsa.recover(TEST_MESSAGE, signature)).should.equal( - '0x0000000000000000000000000000000000000000'); + (await this.ecdsa.recover(TEST_MESSAGE, signature)).should.equal(ZERO_ADDRESS); }); }); @@ -41,8 +41,7 @@ contract('ECDSA', function ([_, anyone]) { // The only valid values are 0, 1, 27 and 28. const version = '02'; const signature = signatureWithoutVersion + version; - (await this.ecdsa.recover(TEST_MESSAGE, signature)).should.equal( - '0x0000000000000000000000000000000000000000'); + (await this.ecdsa.recover(TEST_MESSAGE, signature)).should.equal(ZERO_ADDRESS); }); }); }); @@ -52,16 +51,15 @@ contract('ECDSA', function ([_, anyone]) { // eslint-disable-next-line max-len const signatureWithoutVersion = '0x331fe75a821c982f9127538858900d87d3ec1f9f737338ad67cad133fa48feff48e6fa0c18abc62e42820f05943e47af3e9fbe306ce74d64094bdf1691ee53e0'; - context('with 01 as version value (wrong)', function () { - it('works', async function () { + context('with 01 as version value', function () { + it('returns 0', async function () { const version = '01'; const signature = signatureWithoutVersion + version; - (await this.ecdsa.recover(TEST_MESSAGE, signature)).should.equal( - '0x0000000000000000000000000000000000000000'); + (await this.ecdsa.recover(TEST_MESSAGE, signature)).should.equal(ZERO_ADDRESS); }); }); - context('with 28 signature', function () { + context('with 28 as version value', function () { it('works', async function () { const version = '1c'; // 28 = 1c. const signature = signatureWithoutVersion + version; @@ -75,8 +73,7 @@ contract('ECDSA', function ([_, anyone]) { // The only valid values are 0, 1, 27 and 28. const version = '02'; const signature = signatureWithoutVersion + version; - (await this.ecdsa.recover(TEST_MESSAGE, signature)).should.equal( - '0x0000000000000000000000000000000000000000'); + (await this.ecdsa.recover(TEST_MESSAGE, signature)).should.equal(ZERO_ADDRESS); }); }); }); @@ -85,7 +82,7 @@ contract('ECDSA', function ([_, anyone]) { context('with correct signature', function () { it('returns signer address', async function () { // Create the signature - const signature = fixSignature(await signMessage(anyone, TEST_MESSAGE)); + const signature = fixSignature(await web3.eth.sign(TEST_MESSAGE, anyone)); // Recover the signer address from the generated message and signature. (await this.ecdsa.recover( @@ -98,23 +95,23 @@ contract('ECDSA', function ([_, anyone]) { context('with wrong signature', function () { it('does not return signer address', async function () { // Create the signature - const signature = await signMessage(anyone, TEST_MESSAGE); + const signature = await web3.eth.sign(TEST_MESSAGE, anyone); // Recover the signer address from the generated message and wrong signature. (await this.ecdsa.recover(WRONG_MESSAGE, signature)).should.not.equal(anyone); }); }); }); - }); - context('with small hash', function () { - // @TODO - remove `skip` once we upgrade to solc^0.5 - it.skip('reverts', async function () { - // Create the signature - const signature = await signMessage(anyone, TEST_MESSAGE); - await shouldFail.reverting( - this.ecdsa.recover(TEST_MESSAGE.substring(2), signature) - ); + context('with small hash', function () { + // @TODO - remove `skip` once we upgrade to solc^0.5 + it.skip('reverts', async function () { + // Create the signature + const signature = await web3.eth.sign(TEST_MESSAGE, anyone); + await shouldFail.reverting( + this.ecdsa.recover(TEST_MESSAGE.substring(2), signature) + ); + }); }); }); diff --git a/test/helpers/sign.js b/test/helpers/sign.js index 6398173e36b..38b39a6f657 100644 --- a/test/helpers/sign.js +++ b/test/helpers/sign.js @@ -22,8 +22,8 @@ function fixSignature (signature) { } // signs message in node (ganache auto-applies "Ethereum Signed Message" prefix) -const signMessage = (signer, messageHex = '0x') => { - return web3.eth.sign(messageHex, signer); +async function signMessage (signer, messageHex = '0x') { + return fixSignature(await web3.eth.sign(messageHex, signer)); }; /** From 15e9136b3ec48c6e38945d013341ecc7bde8a63b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Wed, 6 Mar 2019 16:00:11 -0300 Subject: [PATCH 09/10] Add changelog entry. --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 393bb199bf6..f5e11eada7f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ * Upgraded the minimum compiler version to v0.5.2: this removes many Solidity warnings that were false positives. * `Counter`'s API has been improved, and is now used by `ERC721` (though it is still in `drafts`). * `ERC721`'s transfers are now more gas efficient due to removal of unnecessary `SafeMath` calls. + * `ECDSA`: `recover` no longer accepts malleable signatures (those using upper-range values for `s`, or 0/1 for `v`). ([#1622](https://github.com/OpenZeppelin/openzeppelin-solidity/pull/1622)) * Fixed variable shadowing issues. ### Bugfixes: From 93993ceadef0ebe2d88b5620eaa889deb10fae84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Thu, 7 Mar 2019 12:19:09 -0300 Subject: [PATCH 10/10] Add high-s check test. --- contracts/cryptography/ECDSA.sol | 2 +- test/cryptography/ECDSA.test.js | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/contracts/cryptography/ECDSA.sol b/contracts/cryptography/ECDSA.sol index 26b81114e3b..09e4e0c4ec1 100644 --- a/contracts/cryptography/ECDSA.sol +++ b/contracts/cryptography/ECDSA.sol @@ -42,7 +42,7 @@ library ECDSA { // with 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141 - s1 and flip v from 27 to 28 or // vice versa. If your library also generates signatures with 0/1 for v instead 27/28, add 27 to v to accept // these malleable signatures as well. - if (uint256(s) > uint256(0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0)) { + if (uint256(s) > 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0) { return address(0); } diff --git a/test/cryptography/ECDSA.test.js b/test/cryptography/ECDSA.test.js index 76bb9c43bab..57364615923 100644 --- a/test/cryptography/ECDSA.test.js +++ b/test/cryptography/ECDSA.test.js @@ -78,6 +78,16 @@ contract('ECDSA', function ([_, anyone]) { }); }); + context('with high-s value signature', function () { + it('returns 0', async function () { + const message = '0xb94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9'; + // eslint-disable-next-line max-len + const highSSignature = '0xe742ff452d41413616a5bf43fe15dd88294e983d3d36206c2712f39083d638bde0a0fc89be718fbc1033e1d30d78be1c68081562ed2e97af876f286f3453231d1b'; + + (await this.ecdsa.recover(message, highSSignature)).should.equal(ZERO_ADDRESS); + }); + }); + context('using web3.eth.sign', function () { context('with correct signature', function () { it('returns signer address', async function () {