diff options
Diffstat (limited to 'packages/contracts')
4 files changed, 51 insertions, 132 deletions
diff --git a/packages/contracts/src/2.0.0/examples/Whitelist/Whitelist.sol b/packages/contracts/src/2.0.0/examples/Whitelist/Whitelist.sol index 60cac26ea..e4e25038c 100644 --- a/packages/contracts/src/2.0.0/examples/Whitelist/Whitelist.sol +++ b/packages/contracts/src/2.0.0/examples/Whitelist/Whitelist.sol @@ -37,7 +37,7 @@ contract Whitelist is bytes internal TX_ORIGIN_SIGNATURE; // solhint-enable var-name-mixedcase - byte constant internal VALIDATOR_SIGNATURE_BYTE = "\x06"; + byte constant internal VALIDATOR_SIGNATURE_BYTE = "\x05"; constructor (address _exchange) public diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol index 017da742e..f30adcdb8 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol @@ -48,14 +48,16 @@ contract MixinSignatureValidator is ) external { - require( - isValidSignature( - hash, - signerAddress, - signature - ), - "INVALID_SIGNATURE" - ); + if (signerAddress != msg.sender) { + require( + isValidSignature( + hash, + signerAddress, + signature + ), + "INVALID_SIGNATURE" + ); + } preSigned[hash][signerAddress] = true; } @@ -172,22 +174,6 @@ contract MixinSignatureValidator is isValid = signerAddress == recovered; return isValid; - // Implicitly signed by caller. - // The signer has initiated the call. In the case of non-contract - // accounts it means the transaction itself was signed. - // Example: let's say for a particular operation three signatures - // A, B and C are required. To submit the transaction, A and B can - // give a signature to C, who can then submit the transaction using - // `Caller` for his own signature. Or A and C can sign and B can - // submit using `Caller`. Having `Caller` allows this flexibility. - } else if (signatureType == SignatureType.Caller) { - require( - signature.length == 0, - "LENGTH_0_REQUIRED" - ); - isValid = signerAddress == msg.sender; - return isValid; - // Signature verified by wallet contract. // If used with an order, the maker of the order is the wallet contract. } else if (signatureType == SignatureType.Wallet) { @@ -225,34 +211,6 @@ contract MixinSignatureValidator is } else if (signatureType == SignatureType.PreSigned) { isValid = preSigned[hash][signerAddress]; return isValid; - - // Signature from Trezor hardware wallet. - // It differs from web3.eth_sign in the encoding of message length - // (Bitcoin varint encoding vs ascii-decimal, the latter is not - // self-terminating which leads to ambiguities). - // See also: - // https://en.bitcoin.it/wiki/Protocol_documentation#Variable_length_integer - // https://github.com/trezor/trezor-mcu/blob/master/firmware/ethereum.c#L602 - // https://github.com/trezor/trezor-mcu/blob/master/firmware/crypto.c#L36 - } else if (signatureType == SignatureType.Trezor) { - require( - signature.length == 65, - "LENGTH_65_REQUIRED" - ); - v = uint8(signature[0]); - r = signature.readBytes32(1); - s = signature.readBytes32(33); - recovered = ecrecover( - keccak256(abi.encodePacked( - "\x19Ethereum Signed Message:\n\x20", - hash - )), - v, - r, - s - ); - isValid = signerAddress == recovered; - return isValid; } // Anything else is illegal (We do not return false because diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MSignatureValidator.sol b/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MSignatureValidator.sol index 75fe9ec46..1fe88b908 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MSignatureValidator.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MSignatureValidator.sol @@ -36,12 +36,10 @@ contract MSignatureValidator is Invalid, // 0x01 EIP712, // 0x02 EthSign, // 0x03 - Caller, // 0x04 - Wallet, // 0x05 - Validator, // 0x06 - PreSigned, // 0x07 - Trezor, // 0x08 - NSignatureTypes // 0x09, number of signature types. Always leave at end. + Wallet, // 0x04 + Validator, // 0x05 + PreSigned, // 0x06 + NSignatureTypes // 0x07, number of signature types. Always leave at end. } /// @dev Verifies signature using logic defined by Wallet contract. diff --git a/packages/contracts/test/exchange/signature_validator.ts b/packages/contracts/test/exchange/signature_validator.ts index 5b64d853c..da2febfd8 100644 --- a/packages/contracts/test/exchange/signature_validator.ts +++ b/packages/contracts/test/exchange/signature_validator.ts @@ -281,32 +281,6 @@ describe('MixinSignatureValidator', () => { expect(isValidSignature).to.be.false(); }); - it('should return true when SignatureType=Caller and signer is caller', async () => { - const signature = ethUtil.toBuffer(`0x${SignatureType.Caller}`); - const signatureHex = ethUtil.bufferToHex(signature); - const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( - orderHashHex, - signerAddress, - signatureHex, - { from: signerAddress }, - ); - expect(isValidSignature).to.be.true(); - }); - - it('should return false when SignatureType=Caller and signer is not caller', async () => { - const signature = ethUtil.toBuffer(`0x${SignatureType.Caller}`); - const signatureHex = ethUtil.bufferToHex(signature); - const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( - orderHashHex, - signerAddress, - signatureHex, - { from: notSignerAddress }, - ); - expect(isValidSignature).to.be.false(); - }); - it('should return true when SignatureType=Wallet and signature is valid', async () => { // Create EIP712 signature const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); @@ -440,53 +414,6 @@ describe('MixinSignatureValidator', () => { expect(isValidSignature).to.be.false(); }); - it('should return true when SignatureType=Trezor and signature is valid', async () => { - // Create Trezor signature - const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - const orderHashWithTrezorPrefixHex = signatureUtils.addSignedMessagePrefix(orderHashHex, SignerType.Trezor); - const orderHashWithTrezorPrefixBuffer = ethUtil.toBuffer(orderHashWithTrezorPrefixHex); - const ecSignature = ethUtil.ecsign(orderHashWithTrezorPrefixBuffer, signerPrivateKey); - // Create 0x signature from Trezor signature - const signature = Buffer.concat([ - ethUtil.toBuffer(ecSignature.v), - ecSignature.r, - ecSignature.s, - ethUtil.toBuffer(`0x${SignatureType.Trezor}`), - ]); - const signatureHex = ethUtil.bufferToHex(signature); - // Validate signature - const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( - orderHashHex, - signerAddress, - signatureHex, - ); - expect(isValidSignature).to.be.true(); - }); - - it('should return false when SignatureType=Trezor and signature is invalid', async () => { - // Create Trezor signature - const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - const orderHashWithTrezorPrefixHex = signatureUtils.addSignedMessagePrefix(orderHashHex, SignerType.Trezor); - const orderHashWithTrezorPrefixBuffer = ethUtil.toBuffer(orderHashWithTrezorPrefixHex); - const ecSignature = ethUtil.ecsign(orderHashWithTrezorPrefixBuffer, signerPrivateKey); - // Create 0x signature from Trezor signature - const signature = Buffer.concat([ - ethUtil.toBuffer(ecSignature.v), - ecSignature.r, - ecSignature.s, - ethUtil.toBuffer(`0x${SignatureType.Trezor}`), - ]); - const signatureHex = ethUtil.bufferToHex(signature); - // Validate signature. - // This will fail because `signerAddress` signed the message, but we're passing in `notSignerAddress` - const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( - orderHashHex, - notSignerAddress, - signatureHex, - ); - expect(isValidSignature).to.be.false(); - }); - it('should return true when SignatureType=Presigned and signer has presigned hash', async () => { // Presign hash const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); @@ -520,6 +447,42 @@ describe('MixinSignatureValidator', () => { ); expect(isValidSignature).to.be.false(); }); + + it('should return true when message was signed by a Trezor One (firmware version 1.6.2)', async () => { + // messageHash translates to 0x2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b + const messageHash = ethUtil.bufferToHex(ethUtil.toBuffer('++++++++++++++++++++++++++++++++')); + const signer = '0xc28b145f10f0bcf0fc000e778615f8fd73490bad'; + const v = ethUtil.toBuffer('0x1c'); + const r = ethUtil.toBuffer('0x7b888b596ccf87f0bacab0dcb483124973f7420f169b4824d7a12534ac1e9832'); + const s = ethUtil.toBuffer('0x0c8e14f7edc01459e13965f1da56e0c23ed11e2cca932571eee1292178f90424'); + const trezorSignatureType = ethUtil.toBuffer(`0x${SignatureType.EthSign}`); + const signature = Buffer.concat([v, r, s, trezorSignatureType]); + const signatureHex = ethUtil.bufferToHex(signature); + const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( + messageHash, + signer, + signatureHex, + ); + expect(isValidSignature).to.be.true(); + }); + + it('should return true when message was signed by a Trezor Model T (firmware version 2.0.7)', async () => { + // messageHash translates to 0x2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b + const messageHash = ethUtil.bufferToHex(ethUtil.toBuffer('++++++++++++++++++++++++++++++++')); + const signer = '0x98ce6d9345e8ffa7d99ee0822272fae9d2c0e895'; + const v = ethUtil.toBuffer('0x1c'); + const r = ethUtil.toBuffer('0x423b71062c327f0ec4fe199b8da0f34185e59b4c1cb4cc23df86cac4a601fb3f'); + const s = ethUtil.toBuffer('0x53810d6591b5348b7ee08ee812c874b0fdfb942c9849d59512c90e295221091f'); + const trezorSignatureType = ethUtil.toBuffer(`0x${SignatureType.EthSign}`); + const signature = Buffer.concat([v, r, s, trezorSignatureType]); + const signatureHex = ethUtil.bufferToHex(signature); + const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( + messageHash, + signer, + signatureHex, + ); + expect(isValidSignature).to.be.true(); + }); }); describe('setSignatureValidatorApproval', () => { |