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 --- packages/contracts/test/exchange/core.ts | 59 +++++++++++++++++++++- .../contracts/test/exchange/signature_validator.ts | 22 ++++---- 2 files changed, 70 insertions(+), 11 deletions(-) (limited to 'packages/contracts/test/exchange') diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts index bc2bad749..d3f9b95af 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 { TestStaticCallContract } from '../../generated_contract_wrappers/test_static_call'; 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: TestStaticCallContract; + let maliciousValidator: TestStaticCallContract; let signedOrder: SignedOrder; let erc20Balances: ERC20BalancesByOwner; @@ -109,6 +112,12 @@ describe('Exchange core', () => { constants.AWAIT_TRANSACTION_MINED_MS, ); + maliciousWallet = maliciousValidator = await TestStaticCallContract.deployFrom0xArtifactAsync( + artifacts.TestStaticCall, + provider, + txDefaults, + ); + defaultMakerAssetAddress = erc20TokenA.address; defaultTakerAssetAddress = erc20TokenB.address; @@ -161,6 +170,54 @@ 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.InvalidOrderSignature, + ); + }); + + 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 = await orderFactory.newSignedOrderAsync({ + makerAddress: maliciousValidator.address, + }); + signedOrder.signature = `${maliciousValidator.address}0${SignatureType.Validator}`; + await expectTransactionFailedAsync( + exchangeWrapper.fillOrderAsync(signedOrder, takerAddress), + RevertReason.InvalidOrderSignature, + ); + }); }); 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 0e2967bc7..75656d992 100644 --- a/packages/contracts/test/exchange/signature_validator.ts +++ b/packages/contracts/test/exchange/signature_validator.ts @@ -352,7 +352,7 @@ describe('MixinSignatureValidator', () => { expect(isValidSignature).to.be.false(); }); - it('should not allow `isValidSignature` to update state when SignatureType=Wallet', async () => { + it('should return false when `isValidSignature` attempts to update state and not allow state to be updated when SignatureType=Wallet', async () => { // Create EIP712 signature const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); const orderHashBuffer = ethUtil.toBuffer(orderHashHex); @@ -366,13 +366,12 @@ describe('MixinSignatureValidator', () => { ]); const signatureHex = ethUtil.bufferToHex(signature); // Validate signature - await expectContractCallFailedWithoutReasonAsync( - signatureValidator.publicIsValidSignature.callAsync( - orderHashHex, - maliciousWallet.address, - signatureHex, - ), + const isValid = await signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + maliciousWallet.address, + signatureHex, ); + expect(isValid).to.be.equal(false); }); it('should return true when SignatureType=Validator, signature is valid and validator is approved', async () => { @@ -405,15 +404,18 @@ describe('MixinSignatureValidator', () => { expect(isValidSignature).to.be.false(); }); - it('should not allow `isValidSignature` to update state when SignatureType=Validator', async () => { + it('should return false when `isValidSignature` attempts to update state and not allow state to be updated 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), + const isValid = await signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + maliciousValidator.address, + signatureHex, ); + expect(isValid).to.be.equal(false); }); it('should return false when SignatureType=Validator, signature is valid and validator is not approved', async () => { // Set approval of signature validator to false -- cgit v1.2.3