From c5f8b9c2d2b1eed5789b40c4df07e98afe0431ab Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Thu, 23 Aug 2018 17:25:22 -0700 Subject: Add pragma experimental v0.5.0 to SignatureValidator and add tests --- .../protocol/Exchange/MixinSignatureValidator.sol | 1 + .../TestSignatureValidator.sol | 1 + .../2.0.0/test/TestStaticCall/TestStaticCall.sol | 64 ++++++++++++++++++++++ 3 files changed, 66 insertions(+) create mode 100644 packages/contracts/src/2.0.0/test/TestStaticCall/TestStaticCall.sol (limited to 'packages/contracts/src') 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 44de54817..fd655e757 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol @@ -17,6 +17,7 @@ */ pragma solidity 0.4.24; +pragma experimental "v0.5.0"; import "../../utils/LibBytes/LibBytes.sol"; import "./mixins/MSignatureValidator.sol"; diff --git a/packages/contracts/src/2.0.0/test/TestSignatureValidator/TestSignatureValidator.sol b/packages/contracts/src/2.0.0/test/TestSignatureValidator/TestSignatureValidator.sol index e1a610469..3845b7b9e 100644 --- a/packages/contracts/src/2.0.0/test/TestSignatureValidator/TestSignatureValidator.sol +++ b/packages/contracts/src/2.0.0/test/TestSignatureValidator/TestSignatureValidator.sol @@ -17,6 +17,7 @@ */ pragma solidity 0.4.24; +pragma experimental "v0.5.0"; import "../../protocol/Exchange/MixinSignatureValidator.sol"; import "../../protocol/Exchange/MixinTransactions.sol"; diff --git a/packages/contracts/src/2.0.0/test/TestStaticCall/TestStaticCall.sol b/packages/contracts/src/2.0.0/test/TestStaticCall/TestStaticCall.sol new file mode 100644 index 000000000..be24e2f37 --- /dev/null +++ b/packages/contracts/src/2.0.0/test/TestStaticCall/TestStaticCall.sol @@ -0,0 +1,64 @@ +/* + + Copyright 2018 ZeroEx Intl. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + +*/ + +pragma solidity 0.4.24; + + +contract TestStaticCall { + + uint256 internal state = 1; + + /// @dev Updates state and returns true. Intended to be used with `Validator` signature type. + /// @param hash Message hash that is signed. + /// @param signerAddress Address that should have signed the given hash. + /// @param signature Proof of signing. + /// @return Validity of order signature. + function isValidSignature( + bytes32 hash, + address signerAddress, + bytes signature + ) + external + returns (bool isValid) + { + updateState(); + return true; + } + + /// @dev Updates state and returns true. Intended to be used with `Wallet` signature type. + /// @param hash Message hash that is signed. + /// @param signature Proof of signing. + /// @return Validity of order signature. + function isValidSignature( + bytes32 hash, + bytes signature + ) + external + returns (bool isValid) + { + updateState(); + return true; + } + + /// @dev Increments state variable. + function updateState() + internal + { + state++; + } +} -- cgit From 0a1ae2c31139e446b2fb3b7f03caf371c6668ae2 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Fri, 24 Aug 2018 00:19:06 -0700 Subject: Remove pragma experimental v0.5.0 and use staticcall is assembly --- .../protocol/Exchange/MixinSignatureValidator.sol | 84 +++++++++++++++++++++- .../Exchange/mixins/MSignatureValidator.sol | 31 ++++++++ .../TestSignatureValidator.sol | 1 - .../2.0.0/test/TestStaticCall/TestStaticCall.sol | 16 +++++ 4 files changed, 128 insertions(+), 4 deletions(-) (limited to 'packages/contracts/src') 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 fd655e757..6bfbb5107 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol @@ -17,7 +17,6 @@ */ pragma solidity 0.4.24; -pragma experimental "v0.5.0"; import "../../utils/LibBytes/LibBytes.sol"; import "./mixins/MSignatureValidator.sol"; @@ -192,7 +191,11 @@ contract MixinSignatureValidator is // Signature verified by wallet contract. // If used with an order, the maker of the order is the wallet contract. } else if (signatureType == SignatureType.Wallet) { - isValid = IWallet(signerAddress).isValidSignature(hash, signature); + isValid = isValidWalletSignature( + hash, + signerAddress, + signature + ); return isValid; // Signature verified by validator contract. @@ -210,7 +213,8 @@ contract MixinSignatureValidator is if (!allowedValidators[signerAddress][validatorAddress]) { return false; } - isValid = IValidator(validatorAddress).isValidSignature( + isValid = isValidValidatorSignature( + validatorAddress, hash, signerAddress, signature @@ -258,4 +262,78 @@ contract MixinSignatureValidator is // signature was invalid.) revert("SIGNATURE_UNSUPPORTED"); } + + /// @dev Verifies signature using logic defined by Wallet contract. + /// @param hash Any 32 byte hash. + /// @param walletAddress Address that should have signed the given hash + /// and defines its own signature verification method. + /// @param signature Proof that the hash has been signed by signer. + /// @return True if signature is valid for given wallet.. + function isValidWalletSignature( + bytes32 hash, + address walletAddress, + bytes signature + ) + internal + view + returns (bool isValid) + { + bytes memory calldata = abi.encodeWithSelector( + IWallet(walletAddress).isValidSignature.selector, + hash, + signature + ); + assembly { + let cdStart := add(calldata, 32) + let success := staticcall( + gas, // forward all gas + walletAddress, // address of Wallet contract + cdStart, // pointer to start of input + mload(calldata), // length of input + cdStart, // write input over output + 32 // output size is 32 bytes + ) + // Signature is valid if call did not revert and returned true + isValid := and(success, mload(cdStart)) + } + return isValid; + } + + /// @dev Verifies signature using logic defined by Validator contract. + /// @param validatorAddress Address of validator contract. + /// @param hash Any 32 byte hash. + /// @param signerAddress Address that should have signed the given hash. + /// @param signature Proof that the hash has been signed by signer. + /// @return True if the address recovered from the provided signature matches the input signer address. + function isValidValidatorSignature( + address validatorAddress, + bytes32 hash, + address signerAddress, + bytes signature + ) + internal + view + returns (bool isValid) + { + bytes memory calldata = abi.encodeWithSelector( + IValidator(signerAddress).isValidSignature.selector, + hash, + signerAddress, + signature + ); + assembly { + let cdStart := add(calldata, 32) + let success := staticcall( + gas, // forward all gas + validatorAddress, // address of Validator contract + cdStart, // pointer to start of input + mload(calldata), // length of input + cdStart, // write input over output + 32 // output size is 32 bytes + ) + // Signature is valid if call did not revert and returned true + isValid := and(success, mload(cdStart)) + } + return isValid; + } } 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 f14f2ba00..75fe9ec46 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 @@ -43,4 +43,35 @@ contract MSignatureValidator is Trezor, // 0x08 NSignatureTypes // 0x09, number of signature types. Always leave at end. } + + /// @dev Verifies signature using logic defined by Wallet contract. + /// @param hash Any 32 byte hash. + /// @param walletAddress Address that should have signed the given hash + /// and defines its own signature verification method. + /// @param signature Proof that the hash has been signed by signer. + /// @return True if the address recovered from the provided signature matches the input signer address. + function isValidWalletSignature( + bytes32 hash, + address walletAddress, + bytes signature + ) + internal + view + returns (bool isValid); + + /// @dev Verifies signature using logic defined by Validator contract. + /// @param validatorAddress Address of validator contract. + /// @param hash Any 32 byte hash. + /// @param signerAddress Address that should have signed the given hash. + /// @param signature Proof that the hash has been signed by signer. + /// @return True if the address recovered from the provided signature matches the input signer address. + function isValidValidatorSignature( + address validatorAddress, + bytes32 hash, + address signerAddress, + bytes signature + ) + internal + view + returns (bool isValid); } diff --git a/packages/contracts/src/2.0.0/test/TestSignatureValidator/TestSignatureValidator.sol b/packages/contracts/src/2.0.0/test/TestSignatureValidator/TestSignatureValidator.sol index 3845b7b9e..e1a610469 100644 --- a/packages/contracts/src/2.0.0/test/TestSignatureValidator/TestSignatureValidator.sol +++ b/packages/contracts/src/2.0.0/test/TestSignatureValidator/TestSignatureValidator.sol @@ -17,7 +17,6 @@ */ pragma solidity 0.4.24; -pragma experimental "v0.5.0"; import "../../protocol/Exchange/MixinSignatureValidator.sol"; import "../../protocol/Exchange/MixinTransactions.sol"; diff --git a/packages/contracts/src/2.0.0/test/TestStaticCall/TestStaticCall.sol b/packages/contracts/src/2.0.0/test/TestStaticCall/TestStaticCall.sol index be24e2f37..5a9581f51 100644 --- a/packages/contracts/src/2.0.0/test/TestStaticCall/TestStaticCall.sol +++ b/packages/contracts/src/2.0.0/test/TestStaticCall/TestStaticCall.sol @@ -18,6 +18,8 @@ pragma solidity 0.4.24; +import "../../tokens/ERC20Token/IERC20Token.sol"; + contract TestStaticCall { @@ -55,6 +57,20 @@ contract TestStaticCall { return true; } + /// @dev Approves an ERC20 token to spend tokens from this address. + /// @param token Address of ERC20 token. + /// @param spender Address that will spend tokens. + /// @param value Amount of tokens spender is approved to spend. + function approveERC20( + address token, + address spender, + uint256 value + ) + external + { + IERC20Token(token).approve(spender, value); + } + /// @dev Increments state variable. function updateState() internal -- cgit From 681ed822eca91085300e85e136ddcf4abd90495c Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Fri, 24 Aug 2018 09:24:33 -0700 Subject: Revert if undefined function called in AssetProxies --- packages/contracts/src/2.0.0/protocol/AssetProxy/ERC20Proxy.sol | 3 +++ packages/contracts/src/2.0.0/protocol/AssetProxy/ERC721Proxy.sol | 3 +++ 2 files changed, 6 insertions(+) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC20Proxy.sol index b5cec6b64..004c3892d 100644 --- a/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC20Proxy.sol +++ b/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC20Proxy.sol @@ -118,6 +118,9 @@ contract ERC20Proxy is mstore(96, 0) revert(0, 100) } + + // Revert if undefined function is called + revert(0, 0) } } diff --git a/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC721Proxy.sol index 6a70c9f60..9d0bc0f74 100644 --- a/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC721Proxy.sol @@ -152,6 +152,9 @@ contract ERC721Proxy is mstore(96, 0) revert(0, 100) } + + // Revert if undefined function is called + revert(0, 0) } } -- cgit From 070eff6f3a2f3775ef72248c3f345c05cd833798 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Fri, 24 Aug 2018 09:27:29 -0700 Subject: Rename TestStaticCall => TestStaticCallReceiver --- .../2.0.0/test/TestStaticCall/TestStaticCall.sol | 80 --------------------- .../TestStaticCallReceiver.sol | 81 ++++++++++++++++++++++ 2 files changed, 81 insertions(+), 80 deletions(-) delete mode 100644 packages/contracts/src/2.0.0/test/TestStaticCall/TestStaticCall.sol create mode 100644 packages/contracts/src/2.0.0/test/TestStaticCallReceiver/TestStaticCallReceiver.sol (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/2.0.0/test/TestStaticCall/TestStaticCall.sol b/packages/contracts/src/2.0.0/test/TestStaticCall/TestStaticCall.sol deleted file mode 100644 index 5a9581f51..000000000 --- a/packages/contracts/src/2.0.0/test/TestStaticCall/TestStaticCall.sol +++ /dev/null @@ -1,80 +0,0 @@ -/* - - Copyright 2018 ZeroEx Intl. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - -*/ - -pragma solidity 0.4.24; - -import "../../tokens/ERC20Token/IERC20Token.sol"; - - -contract TestStaticCall { - - uint256 internal state = 1; - - /// @dev Updates state and returns true. Intended to be used with `Validator` signature type. - /// @param hash Message hash that is signed. - /// @param signerAddress Address that should have signed the given hash. - /// @param signature Proof of signing. - /// @return Validity of order signature. - function isValidSignature( - bytes32 hash, - address signerAddress, - bytes signature - ) - external - returns (bool isValid) - { - updateState(); - return true; - } - - /// @dev Updates state and returns true. Intended to be used with `Wallet` signature type. - /// @param hash Message hash that is signed. - /// @param signature Proof of signing. - /// @return Validity of order signature. - function isValidSignature( - bytes32 hash, - bytes signature - ) - external - returns (bool isValid) - { - updateState(); - return true; - } - - /// @dev Approves an ERC20 token to spend tokens from this address. - /// @param token Address of ERC20 token. - /// @param spender Address that will spend tokens. - /// @param value Amount of tokens spender is approved to spend. - function approveERC20( - address token, - address spender, - uint256 value - ) - external - { - IERC20Token(token).approve(spender, value); - } - - /// @dev Increments state variable. - function updateState() - internal - { - state++; - } -} diff --git a/packages/contracts/src/2.0.0/test/TestStaticCallReceiver/TestStaticCallReceiver.sol b/packages/contracts/src/2.0.0/test/TestStaticCallReceiver/TestStaticCallReceiver.sol new file mode 100644 index 000000000..41aab01c8 --- /dev/null +++ b/packages/contracts/src/2.0.0/test/TestStaticCallReceiver/TestStaticCallReceiver.sol @@ -0,0 +1,81 @@ +/* + + Copyright 2018 ZeroEx Intl. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + +*/ + +pragma solidity 0.4.24; + +import "../../tokens/ERC20Token/IERC20Token.sol"; + + +// solhint-disable no-unused-vars +contract TestStaticCallReceiver { + + uint256 internal state = 1; + + /// @dev Updates state and returns true. Intended to be used with `Validator` signature type. + /// @param hash Message hash that is signed. + /// @param signerAddress Address that should have signed the given hash. + /// @param signature Proof of signing. + /// @return Validity of order signature. + function isValidSignature( + bytes32 hash, + address signerAddress, + bytes signature + ) + external + returns (bool isValid) + { + updateState(); + return true; + } + + /// @dev Updates state and returns true. Intended to be used with `Wallet` signature type. + /// @param hash Message hash that is signed. + /// @param signature Proof of signing. + /// @return Validity of order signature. + function isValidSignature( + bytes32 hash, + bytes signature + ) + external + returns (bool isValid) + { + updateState(); + return true; + } + + /// @dev Approves an ERC20 token to spend tokens from this address. + /// @param token Address of ERC20 token. + /// @param spender Address that will spend tokens. + /// @param value Amount of tokens spender is approved to spend. + function approveERC20( + address token, + address spender, + uint256 value + ) + external + { + IERC20Token(token).approve(spender, value); + } + + /// @dev Increments state variable. + function updateState() + internal + { + state++; + } +} -- cgit From 6a9669a409a61c4645af43f39a4e4a0761354e32 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Fri, 24 Aug 2018 14:06:46 -0700 Subject: Rethrow Wallet and Validator errors --- .../protocol/Exchange/MixinSignatureValidator.sol | 32 +++++++++++++++++++--- 1 file changed, 28 insertions(+), 4 deletions(-) (limited to 'packages/contracts/src') 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 6bfbb5107..017da742e 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol @@ -293,8 +293,20 @@ contract MixinSignatureValidator is cdStart, // write input over output 32 // output size is 32 bytes ) - // Signature is valid if call did not revert and returned true - isValid := and(success, mload(cdStart)) + + switch success + case 0 { + // Revert with `Error("WALLET_ERROR")` + mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000) + mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000) + mstore(64, 0x0000000c57414c4c45545f4552524f5200000000000000000000000000000000) + mstore(96, 0) + revert(0, 100) + } + case 1 { + // Signature is valid if call did not revert and returned true + isValid := mload(cdStart) + } } return isValid; } @@ -331,8 +343,20 @@ contract MixinSignatureValidator is cdStart, // write input over output 32 // output size is 32 bytes ) - // Signature is valid if call did not revert and returned true - isValid := and(success, mload(cdStart)) + + switch success + case 0 { + // Revert with `Error("VALIDATOR_ERROR")` + mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000) + mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000) + mstore(64, 0x0000000f56414c494441544f525f4552524f5200000000000000000000000000) + mstore(96, 0) + revert(0, 100) + } + case 1 { + // Signature is valid if call did not revert and returned true + isValid := mload(cdStart) + } } return isValid; } -- cgit