diff options
Diffstat (limited to 'packages')
7 files changed, 123 insertions, 2 deletions
diff --git a/packages/contracts/compiler.json b/packages/contracts/compiler.json index 16524231c..741614f2c 100644 --- a/packages/contracts/compiler.json +++ b/packages/contracts/compiler.json @@ -47,6 +47,7 @@ "TestLibs", "TestExchangeInternals", "TestSignatureValidator", + "TestStaticCall", "TokenRegistry", "Validator", "Wallet", diff --git a/packages/contracts/package.json b/packages/contracts/package.json index 10ab09767..6fd5a864b 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|TestStaticCall|Validator|Wallet|TokenRegistry|Whitelist|WETH9|ZRXToken).json" }, "repository": { "type": "git", 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++; + } +} diff --git a/packages/contracts/test/exchange/signature_validator.ts b/packages/contracts/test/exchange/signature_validator.ts index cd4f3d6f3..0e2967bc7 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 { TestStaticCallContract } from '../../generated_contract_wrappers/test_static_call'; 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: TestStaticCallContract; + let maliciousValidator: TestStaticCallContract; let signerAddress: string; let signerPrivateKey: Buffer; let notSignerAddress: string; @@ -65,6 +68,11 @@ describe('MixinSignatureValidator', () => { txDefaults, signerAddress, ); + maliciousWallet = maliciousValidator = await TestStaticCallContract.deployFrom0xArtifactAsync( + artifacts.TestStaticCall, + 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 not allow `isValidSignature` to update state when 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); + // Validate signature + await expectContractCallFailedWithoutReasonAsync( + signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + maliciousWallet.address, + signatureHex, + ), + ); + }); + 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,16 @@ describe('MixinSignatureValidator', () => { expect(isValidSignature).to.be.false(); }); + it('should not allow `isValidSignature` to update state when 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 expectContractCallFailedWithoutReasonAsync( + signatureValidator.publicIsValidSignature.callAsync(orderHashHex, signerAddress, signatureHex), + ); + }); 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..1c922a3e4 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 TestStaticCall from '../../artifacts/TestStaticCall.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, + TestStaticCall: (TestStaticCall as any) as ContractArtifact, Validator: (Validator as any) as ContractArtifact, Wallet: (Wallet as any) as ContractArtifact, TokenRegistry: (TokenRegistry as any) as ContractArtifact, |