aboutsummaryrefslogtreecommitdiffstats
path: root/packages/contracts
diff options
context:
space:
mode:
Diffstat (limited to 'packages/contracts')
-rw-r--r--packages/contracts/compiler.json1
-rw-r--r--packages/contracts/package.json3
-rw-r--r--packages/contracts/src/2.0.0/examples/Whitelist/Whitelist.sol2
-rw-r--r--packages/contracts/src/2.0.0/protocol/AssetProxy/ERC20Proxy.sol3
-rw-r--r--packages/contracts/src/2.0.0/protocol/AssetProxy/ERC721Proxy.sol3
-rw-r--r--packages/contracts/src/2.0.0/protocol/Exchange/MixinAssetProxyDispatcher.sol2
-rw-r--r--packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol169
-rw-r--r--packages/contracts/src/2.0.0/protocol/Exchange/mixins/MSignatureValidator.sol41
-rw-r--r--packages/contracts/src/2.0.0/test/TestStaticCallReceiver/TestStaticCallReceiver.sol81
-rw-r--r--packages/contracts/test/asset_proxy/proxies.ts24
-rw-r--r--packages/contracts/test/exchange/core.ts56
-rw-r--r--packages/contracts/test/exchange/signature_validator.ts163
-rw-r--r--packages/contracts/test/utils/artifacts.ts2
13 files changed, 411 insertions, 139 deletions
diff --git a/packages/contracts/compiler.json b/packages/contracts/compiler.json
index 16524231c..27bb69a36 100644
--- a/packages/contracts/compiler.json
+++ b/packages/contracts/compiler.json
@@ -47,6 +47,7 @@
"TestLibs",
"TestExchangeInternals",
"TestSignatureValidator",
+ "TestStaticCallReceiver",
"TokenRegistry",
"Validator",
"Wallet",
diff --git a/packages/contracts/package.json b/packages/contracts/package.json
index 10ab09767..0ead2f43f 100644
--- a/packages/contracts/package.json
+++ b/packages/contracts/package.json
@@ -33,7 +33,8 @@
"lint-contracts": "solhint src/2.0.0/**/**/**/**/*.sol"
},
"config": {
- "abis": "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Receiver|DummyERC721Token|DummyNoReturnERC20Token|ERC20Proxy|ERC721Proxy|Forwarder|Exchange|ExchangeWrapper|IAssetData|IAssetProxy|InvalidERC721Receiver|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|OrderValidator|TestAssetProxyOwner|TestAssetProxyDispatcher|TestConstants|TestExchangeInternals|TestLibBytes|TestLibs|TestSignatureValidator|Validator|Wallet|TokenRegistry|Whitelist|WETH9|ZRXToken).json"
+ "abis":
+ "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Receiver|DummyERC721Token|DummyNoReturnERC20Token|ERC20Proxy|ERC721Proxy|Forwarder|Exchange|ExchangeWrapper|IAssetData|IAssetProxy|InvalidERC721Receiver|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|OrderValidator|TestAssetProxyOwner|TestAssetProxyDispatcher|TestConstants|TestExchangeInternals|TestLibBytes|TestLibs|TestSignatureValidator|TestStaticCallReceiver|Validator|Wallet|TokenRegistry|Whitelist|WETH9|ZRXToken).json"
},
"repository": {
"type": "git",
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/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)
}
}
diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinAssetProxyDispatcher.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinAssetProxyDispatcher.sol
index e9f882194..80475e6e3 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinAssetProxyDispatcher.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinAssetProxyDispatcher.sol
@@ -83,7 +83,7 @@ contract MixinAssetProxyDispatcher is
internal
{
// Do nothing if no amount should be transferred.
- if (amount > 0) {
+ if (amount > 0 && from != to) {
// Ensure assetData length is valid
require(
assetData.length > 3,
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..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,26 +174,14 @@ 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) {
- isValid = IWallet(signerAddress).isValidSignature(hash, signature);
+ isValid = isValidWalletSignature(
+ hash,
+ signerAddress,
+ signature
+ );
return isValid;
// Signature verified by validator contract.
@@ -209,7 +199,8 @@ contract MixinSignatureValidator is
if (!allowedValidators[signerAddress][validatorAddress]) {
return false;
}
- isValid = IValidator(validatorAddress).isValidSignature(
+ isValid = isValidValidatorSignature(
+ validatorAddress,
hash,
signerAddress,
signature
@@ -220,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
@@ -257,4 +220,102 @@ 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
+ )
+
+ 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;
+ }
+
+ /// @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
+ )
+
+ 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;
+ }
}
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..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,11 +36,40 @@ 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.
+ /// @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/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++;
+ }
+}
diff --git a/packages/contracts/test/asset_proxy/proxies.ts b/packages/contracts/test/asset_proxy/proxies.ts
index 76a020222..6031e554d 100644
--- a/packages/contracts/test/asset_proxy/proxies.ts
+++ b/packages/contracts/test/asset_proxy/proxies.ts
@@ -12,7 +12,7 @@ import { ERC20ProxyContract } from '../../generated_contract_wrappers/erc20_prox
import { ERC721ProxyContract } from '../../generated_contract_wrappers/erc721_proxy';
import { IAssetProxyContract } from '../../generated_contract_wrappers/i_asset_proxy';
import { artifacts } from '../utils/artifacts';
-import { expectTransactionFailedAsync } from '../utils/assertions';
+import { expectTransactionFailedAsync, expectTransactionFailedWithoutReasonAsync } from '../utils/assertions';
import { chaiSetup } from '../utils/chai_setup';
import { constants } from '../utils/constants';
import { ERC20Wrapper } from '../utils/erc20_wrapper';
@@ -99,6 +99,17 @@ describe('Asset Transfer Proxies', () => {
await blockchainLifecycle.revertAsync();
});
describe('Transfer Proxy - ERC20', () => {
+ it('should revert if undefined function is called', async () => {
+ const undefinedSelector = '0x01020304';
+ await expectTransactionFailedWithoutReasonAsync(
+ web3Wrapper.sendTransactionAsync({
+ from: owner,
+ to: erc20Proxy.address,
+ value: constants.ZERO_AMOUNT,
+ data: undefinedSelector,
+ }),
+ );
+ });
describe('transferFrom', () => {
it('should successfully transfer tokens', async () => {
// Construct ERC20 asset data
@@ -219,6 +230,17 @@ describe('Asset Transfer Proxies', () => {
});
describe('Transfer Proxy - ERC721', () => {
+ it('should revert if undefined function is called', async () => {
+ const undefinedSelector = '0x01020304';
+ await expectTransactionFailedWithoutReasonAsync(
+ web3Wrapper.sendTransactionAsync({
+ from: owner,
+ to: erc721Proxy.address,
+ value: constants.ZERO_AMOUNT,
+ data: undefinedSelector,
+ }),
+ );
+ });
describe('transferFrom', () => {
it('should successfully transfer tokens', async () => {
// Construct ERC721 asset data
diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts
index bc2bad749..f9d8b7783 100644
--- a/packages/contracts/test/exchange/core.ts
+++ b/packages/contracts/test/exchange/core.ts
@@ -1,6 +1,6 @@
import { BlockchainLifecycle } from '@0xproject/dev-utils';
import { assetDataUtils, orderHashUtils } from '@0xproject/order-utils';
-import { RevertReason, SignedOrder } from '@0xproject/types';
+import { RevertReason, SignatureType, SignedOrder } from '@0xproject/types';
import { BigNumber } from '@0xproject/utils';
import { Web3Wrapper } from '@0xproject/web3-wrapper';
import * as chai from 'chai';
@@ -14,6 +14,7 @@ import { DummyNoReturnERC20TokenContract } from '../../generated_contract_wrappe
import { ERC20ProxyContract } from '../../generated_contract_wrappers/erc20_proxy';
import { ERC721ProxyContract } from '../../generated_contract_wrappers/erc721_proxy';
import { ExchangeCancelEventArgs, ExchangeContract } from '../../generated_contract_wrappers/exchange';
+import { TestStaticCallReceiverContract } from '../../generated_contract_wrappers/test_static_call_receiver';
import { artifacts } from '../utils/artifacts';
import { expectTransactionFailedAsync } from '../utils/assertions';
import { getLatestBlockTimestampAsync, increaseTimeAndMineBlockAsync } from '../utils/block_timestamp';
@@ -44,6 +45,8 @@ describe('Exchange core', () => {
let exchange: ExchangeContract;
let erc20Proxy: ERC20ProxyContract;
let erc721Proxy: ERC721ProxyContract;
+ let maliciousWallet: TestStaticCallReceiverContract;
+ let maliciousValidator: TestStaticCallReceiverContract;
let signedOrder: SignedOrder;
let erc20Balances: ERC20BalancesByOwner;
@@ -109,6 +112,12 @@ describe('Exchange core', () => {
constants.AWAIT_TRANSACTION_MINED_MS,
);
+ maliciousWallet = maliciousValidator = await TestStaticCallReceiverContract.deployFrom0xArtifactAsync(
+ artifacts.TestStaticCallReceiver,
+ provider,
+ txDefaults,
+ );
+
defaultMakerAssetAddress = erc20TokenA.address;
defaultTakerAssetAddress = erc20TokenB.address;
@@ -161,6 +170,51 @@ describe('Exchange core', () => {
RevertReason.OrderUnfillable,
);
});
+
+ it('should revert if `isValidSignature` tries to update state when SignatureType=Wallet', async () => {
+ const maliciousMakerAddress = maliciousWallet.address;
+ await web3Wrapper.awaitTransactionSuccessAsync(
+ await erc20TokenA.setBalance.sendTransactionAsync(
+ maliciousMakerAddress,
+ constants.INITIAL_ERC20_BALANCE,
+ ),
+ constants.AWAIT_TRANSACTION_MINED_MS,
+ );
+ await web3Wrapper.awaitTransactionSuccessAsync(
+ await maliciousWallet.approveERC20.sendTransactionAsync(
+ erc20TokenA.address,
+ erc20Proxy.address,
+ constants.INITIAL_ERC20_ALLOWANCE,
+ ),
+ constants.AWAIT_TRANSACTION_MINED_MS,
+ );
+ signedOrder = await orderFactory.newSignedOrderAsync({
+ makerAddress: maliciousMakerAddress,
+ makerFee: constants.ZERO_AMOUNT,
+ });
+ signedOrder.signature = `0x0${SignatureType.Wallet}`;
+ await expectTransactionFailedAsync(
+ exchangeWrapper.fillOrderAsync(signedOrder, takerAddress),
+ RevertReason.WalletError,
+ );
+ });
+
+ it('should revert if `isValidSignature` tries to update state when SignatureType=Validator', async () => {
+ const isApproved = true;
+ await web3Wrapper.awaitTransactionSuccessAsync(
+ await exchange.setSignatureValidatorApproval.sendTransactionAsync(
+ maliciousValidator.address,
+ isApproved,
+ { from: makerAddress },
+ ),
+ constants.AWAIT_TRANSACTION_MINED_MS,
+ );
+ signedOrder.signature = `${maliciousValidator.address}0${SignatureType.Validator}`;
+ await expectTransactionFailedAsync(
+ exchangeWrapper.fillOrderAsync(signedOrder, takerAddress),
+ RevertReason.ValidatorError,
+ );
+ });
});
describe('Testing exchange of ERC20 tokens with no return values', () => {
diff --git a/packages/contracts/test/exchange/signature_validator.ts b/packages/contracts/test/exchange/signature_validator.ts
index cd4f3d6f3..da2febfd8 100644
--- a/packages/contracts/test/exchange/signature_validator.ts
+++ b/packages/contracts/test/exchange/signature_validator.ts
@@ -9,11 +9,12 @@ import {
TestSignatureValidatorContract,
TestSignatureValidatorSignatureValidatorApprovalEventArgs,
} from '../../generated_contract_wrappers/test_signature_validator';
+import { TestStaticCallReceiverContract } from '../../generated_contract_wrappers/test_static_call_receiver';
import { ValidatorContract } from '../../generated_contract_wrappers/validator';
import { WalletContract } from '../../generated_contract_wrappers/wallet';
import { addressUtils } from '../utils/address_utils';
import { artifacts } from '../utils/artifacts';
-import { expectContractCallFailed } from '../utils/assertions';
+import { expectContractCallFailed, expectContractCallFailedWithoutReasonAsync } from '../utils/assertions';
import { chaiSetup } from '../utils/chai_setup';
import { constants } from '../utils/constants';
import { LogDecoder } from '../utils/log_decoder';
@@ -31,6 +32,8 @@ describe('MixinSignatureValidator', () => {
let signatureValidator: TestSignatureValidatorContract;
let testWallet: WalletContract;
let testValidator: ValidatorContract;
+ let maliciousWallet: TestStaticCallReceiverContract;
+ let maliciousValidator: TestStaticCallReceiverContract;
let signerAddress: string;
let signerPrivateKey: Buffer;
let notSignerAddress: string;
@@ -65,6 +68,11 @@ describe('MixinSignatureValidator', () => {
txDefaults,
signerAddress,
);
+ maliciousWallet = maliciousValidator = await TestStaticCallReceiverContract.deployFrom0xArtifactAsync(
+ artifacts.TestStaticCallReceiver,
+ provider,
+ txDefaults,
+ );
signatureValidatorLogDecoder = new LogDecoder(web3Wrapper);
await web3Wrapper.awaitTransactionSuccessAsync(
await signatureValidator.setSignatureValidatorApproval.sendTransactionAsync(testValidator.address, true, {
@@ -72,6 +80,16 @@ describe('MixinSignatureValidator', () => {
}),
constants.AWAIT_TRANSACTION_MINED_MS,
);
+ await web3Wrapper.awaitTransactionSuccessAsync(
+ await signatureValidator.setSignatureValidatorApproval.sendTransactionAsync(
+ maliciousValidator.address,
+ true,
+ {
+ from: signerAddress,
+ },
+ ),
+ constants.AWAIT_TRANSACTION_MINED_MS,
+ );
const defaultOrderParams = {
...constants.STATIC_ORDER_PARAMS,
@@ -263,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);
@@ -334,6 +326,29 @@ describe('MixinSignatureValidator', () => {
expect(isValidSignature).to.be.false();
});
+ it('should revert when `isValidSignature` attempts to update state and SignatureType=Wallet', async () => {
+ // Create EIP712 signature
+ const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
+ const orderHashBuffer = ethUtil.toBuffer(orderHashHex);
+ const ecSignature = ethUtil.ecsign(orderHashBuffer, signerPrivateKey);
+ // Create 0x signature from EIP712 signature
+ const signature = Buffer.concat([
+ ethUtil.toBuffer(ecSignature.v),
+ ecSignature.r,
+ ecSignature.s,
+ ethUtil.toBuffer(`0x${SignatureType.Wallet}`),
+ ]);
+ const signatureHex = ethUtil.bufferToHex(signature);
+ await expectContractCallFailed(
+ signatureValidator.publicIsValidSignature.callAsync(
+ orderHashHex,
+ maliciousWallet.address,
+ signatureHex,
+ ),
+ RevertReason.WalletError,
+ );
+ });
+
it('should return true when SignatureType=Validator, signature is valid and validator is approved', async () => {
const validatorAddress = ethUtil.toBuffer(`${testValidator.address}`);
const signatureType = ethUtil.toBuffer(`0x${SignatureType.Validator}`);
@@ -364,6 +379,17 @@ describe('MixinSignatureValidator', () => {
expect(isValidSignature).to.be.false();
});
+ it('should revert when `isValidSignature` attempts to update state and SignatureType=Validator', async () => {
+ const validatorAddress = ethUtil.toBuffer(`${maliciousValidator.address}`);
+ const signatureType = ethUtil.toBuffer(`0x${SignatureType.Validator}`);
+ const signature = Buffer.concat([validatorAddress, signatureType]);
+ const signatureHex = ethUtil.bufferToHex(signature);
+ const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
+ await expectContractCallFailed(
+ signatureValidator.publicIsValidSignature.callAsync(orderHashHex, signerAddress, signatureHex),
+ RevertReason.ValidatorError,
+ );
+ });
it('should return false when SignatureType=Validator, signature is valid and validator is not approved', async () => {
// Set approval of signature validator to false
await web3Wrapper.awaitTransactionSuccessAsync(
@@ -388,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);
@@ -468,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', () => {
diff --git a/packages/contracts/test/utils/artifacts.ts b/packages/contracts/test/utils/artifacts.ts
index e8a7585ac..2f6fcef71 100644
--- a/packages/contracts/test/utils/artifacts.ts
+++ b/packages/contracts/test/utils/artifacts.ts
@@ -23,6 +23,7 @@ import * as TestExchangeInternals from '../../artifacts/TestExchangeInternals.js
import * as TestLibBytes from '../../artifacts/TestLibBytes.json';
import * as TestLibs from '../../artifacts/TestLibs.json';
import * as TestSignatureValidator from '../../artifacts/TestSignatureValidator.json';
+import * as TestStaticCallReceiver from '../../artifacts/TestStaticCallReceiver.json';
import * as TokenRegistry from '../../artifacts/TokenRegistry.json';
import * as Validator from '../../artifacts/Validator.json';
import * as Wallet from '../../artifacts/Wallet.json';
@@ -55,6 +56,7 @@ export const artifacts = {
TestLibs: (TestLibs as any) as ContractArtifact,
TestExchangeInternals: (TestExchangeInternals as any) as ContractArtifact,
TestSignatureValidator: (TestSignatureValidator as any) as ContractArtifact,
+ TestStaticCallReceiver: (TestStaticCallReceiver as any) as ContractArtifact,
Validator: (Validator as any) as ContractArtifact,
Wallet: (Wallet as any) as ContractArtifact,
TokenRegistry: (TokenRegistry as any) as ContractArtifact,