aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAmir Bandeali <abandeali1@gmail.com>2018-08-25 05:38:54 +0800
committerGitHub <noreply@github.com>2018-08-25 05:38:54 +0800
commit7351bf0b14e24af78cd67e1c91c18ae0ca078bf1 (patch)
tree4054f02bf577ea213122cf5ad3694fc6586db85f
parent7f36574a57ced8ead0059b90673fe01f97f04827 (diff)
parent6a9669a409a61c4645af43f39a4e4a0761354e32 (diff)
downloaddexon-sol-tools-7351bf0b14e24af78cd67e1c91c18ae0ca078bf1.tar
dexon-sol-tools-7351bf0b14e24af78cd67e1c91c18ae0ca078bf1.tar.gz
dexon-sol-tools-7351bf0b14e24af78cd67e1c91c18ae0ca078bf1.tar.bz2
dexon-sol-tools-7351bf0b14e24af78cd67e1c91c18ae0ca078bf1.tar.lz
dexon-sol-tools-7351bf0b14e24af78cd67e1c91c18ae0ca078bf1.tar.xz
dexon-sol-tools-7351bf0b14e24af78cd67e1c91c18ae0ca078bf1.tar.zst
dexon-sol-tools-7351bf0b14e24af78cd67e1c91c18ae0ca078bf1.zip
Merge pull request #1012 from 0xProject/feature/contracts/staticcall
Use staticcall for external function calls in MixinSignatureValidator
-rw-r--r--packages/contracts/compiler.json1
-rw-r--r--packages/contracts/package.json3
-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/MixinSignatureValidator.sol107
-rw-r--r--packages/contracts/src/2.0.0/protocol/Exchange/mixins/MSignatureValidator.sol31
-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.ts54
-rw-r--r--packages/contracts/test/utils/artifacts.ts2
-rw-r--r--packages/types/CHANGELOG.json9
-rw-r--r--packages/types/src/index.ts2
13 files changed, 370 insertions, 6 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/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/MixinSignatureValidator.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol
index 44de54817..017da742e 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol
@@ -191,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.
@@ -209,7 +213,8 @@ contract MixinSignatureValidator is
if (!allowedValidators[signerAddress][validatorAddress]) {
return false;
}
- isValid = IValidator(validatorAddress).isValidSignature(
+ isValid = isValidValidatorSignature(
+ validatorAddress,
hash,
signerAddress,
signature
@@ -257,4 +262,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..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/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..5b64d853c 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,
@@ -334,6 +352,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 +405,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(
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,
diff --git a/packages/types/CHANGELOG.json b/packages/types/CHANGELOG.json
index fabc80ecf..980a12ad3 100644
--- a/packages/types/CHANGELOG.json
+++ b/packages/types/CHANGELOG.json
@@ -1,5 +1,14 @@
[
{
+ "version": "1.0.1-rc.6",
+ "changes": [
+ {
+ "note": "Add WalletError and ValidatorError revert reasons",
+ "pr": 1012
+ }
+ ]
+ },
+ {
"version": "1.0.1-rc.5",
"changes": [
{
diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts
index 298fa77d4..2831d816d 100644
--- a/packages/types/src/index.ts
+++ b/packages/types/src/index.ts
@@ -220,6 +220,8 @@ export enum RevertReason {
Erc721InvalidSpender = 'ERC721_INVALID_SPENDER',
Erc721ZeroOwner = 'ERC721_ZERO_OWNER',
Erc721InvalidSelector = 'ERC721_INVALID_SELECTOR',
+ WalletError = 'WALLET_ERROR',
+ ValidatorError = 'VALIDATOR_ERROR',
}
export enum StatusCodes {