From ed5ec8a893e9a18814ec957f9fc83b5f17ce2108 Mon Sep 17 00:00:00 2001 From: AugustoL Date: Tue, 11 Jun 2019 12:57:29 -0300 Subject: [PATCH 1/3] feat(contracts): Creating one proxy per ERC827 call - Creates a ERC827 proxy for every call and destroy it after being used. - Proxies addresses are protected against replay attacks. - Reorganization of contracts folder. BREAKING CHANGE: Using dinamyc procies instead of a single proxy contract --- contracts/ERC827/ERC827.sol | 14 +++-- contracts/ERC827/ERC827Migratable.sol | 2 +- contracts/ERC827/ERC827Mock.sol | 14 ----- contracts/ERC827/ERC827Proxy.sol | 15 +++-- .../{ERC20TokenMock.sol => ERC20Mock.sol} | 2 +- contracts/mocks/ERC827Mock.sol | 14 +++++ contracts/mocks/ERC827Receiver.sol | 47 ++++++++++++++ contracts/mocks/ERC827TokenMock.sol | 14 ----- contracts/mocks/MessageHelper.sol | 39 ------------ contracts/utils/Create2.sol | 63 +++++++++++++++++++ 10 files changed, 144 insertions(+), 80 deletions(-) delete mode 100644 contracts/ERC827/ERC827Mock.sol rename contracts/mocks/{ERC20TokenMock.sol => ERC20Mock.sol} (87%) create mode 100644 contracts/mocks/ERC827Mock.sol create mode 100644 contracts/mocks/ERC827Receiver.sol delete mode 100644 contracts/mocks/ERC827TokenMock.sol delete mode 100644 contracts/mocks/MessageHelper.sol create mode 100644 contracts/utils/Create2.sol diff --git a/contracts/ERC827/ERC827.sol b/contracts/ERC827/ERC827.sol index 3f47d4d..0d0928e 100644 --- a/contracts/ERC827/ERC827.sol +++ b/contracts/ERC827/ERC827.sol @@ -3,7 +3,7 @@ pragma solidity ^0.5.2; import "openzeppelin-solidity/contracts/token/ERC20/ERC20.sol"; -import "./ERC827Proxy.sol"; +import "../utils/Create2.sol"; /** @@ -15,13 +15,14 @@ import "./ERC827Proxy.sol"; */ contract ERC827 is ERC20 { - ERC827Proxy public proxy; + bytes public proxyBytecode; + mapping(address => uint256) public nonces; /** * @dev Constructor */ - constructor() public { - proxy = new ERC827Proxy(); + constructor(bytes memory _proxyBytecode) public { + proxyBytecode = _proxyBytecode; } /** @@ -85,11 +86,14 @@ contract ERC827 is ERC20 { * @param _data ABI-encoded contract call to call `_to` address. */ function _call(address _to, bytes memory _data) internal { + bytes32 salt = keccak256(abi.encodePacked(msg.sender, _to, nonces[msg.sender])); + address proxy = Create2.deploy(salt, proxyBytecode); // solium-disable-next-line security/no-call-value, no-unused-vars (bool success, bytes memory data) = address(proxy).call.value(msg.value)( - abi.encodeWithSelector(proxy.callContractFunctionSignature(), _to, _data) + abi.encodeWithSelector(bytes4(keccak256("callContract(address,bytes)")), _to, _data) ); require(success, "Call to external contract failed"); + nonces[msg.sender].add(1); } } diff --git a/contracts/ERC827/ERC827Migratable.sol b/contracts/ERC827/ERC827Migratable.sol index f16aa17..3384b95 100644 --- a/contracts/ERC827/ERC827Migratable.sol +++ b/contracts/ERC827/ERC827Migratable.sol @@ -24,7 +24,7 @@ contract ERC827Migratable is ERC827, Ownable { * be migrated. * @param _erc20Token The address of the erc20 token to be migrated */ - constructor(address _erc20Token) public { + constructor(address _erc20Token, bytes memory proxyBytecode) ERC827(proxyBytecode) public { require(_erc20Token != address(0), "Incorrect ERC20 token address"); erc20Token = IERC20(_erc20Token); } diff --git a/contracts/ERC827/ERC827Mock.sol b/contracts/ERC827/ERC827Mock.sol deleted file mode 100644 index 63a559c..0000000 --- a/contracts/ERC827/ERC827Mock.sol +++ /dev/null @@ -1,14 +0,0 @@ -pragma solidity ^0.5.0; - - -import "./ERC827.sol"; - - -// mock class using ERC827 Token -contract ERC827Mock is ERC827 { - - constructor(address initialAccount, uint256 initialBalance) public ERC827() { - _mint(initialAccount, initialBalance); - } - -} diff --git a/contracts/ERC827/ERC827Proxy.sol b/contracts/ERC827/ERC827Proxy.sol index 422eddd..d9cf5b0 100644 --- a/contracts/ERC827/ERC827Proxy.sol +++ b/contracts/ERC827/ERC827Proxy.sol @@ -2,6 +2,8 @@ pragma solidity ^0.5.2; +import "./IERC827.sol"; + /** * @title ERC827Proxy @@ -11,16 +13,13 @@ pragma solidity ^0.5.2; */ contract ERC827Proxy { - address public token; - bytes4 public callContractFunctionSignature = bytes4( - keccak256("callContract(address,bytes)") - ); + IERC827 public token; /** * @dev constructor */ constructor() public { - token = address(msg.sender); + token = IERC827(msg.sender); } /** @@ -36,8 +35,12 @@ contract ERC827Proxy { "Proxy only can execute calls from the token contract" ); // solium-disable-next-line security/no-call-value, no-unused-vars - (bool success, bytes memory data) = _target .call.value(msg.value)(_data); + (bool success, bytes memory data) = _target.call.value(msg.value)(_data); require(success, "Proxy call failed"); + require(token.balanceOf(address(this)) == 0, "Cant end proxy call with token balance in proxy"); + require(address(this).balance == 0, "Cant end proxy call with eth balance in proxy"); + + selfdestruct(address(0)); return true; } diff --git a/contracts/mocks/ERC20TokenMock.sol b/contracts/mocks/ERC20Mock.sol similarity index 87% rename from contracts/mocks/ERC20TokenMock.sol rename to contracts/mocks/ERC20Mock.sol index efc7e91..29d83da 100644 --- a/contracts/mocks/ERC20TokenMock.sol +++ b/contracts/mocks/ERC20Mock.sol @@ -5,7 +5,7 @@ import "openzeppelin-solidity/contracts/token/ERC20/ERC20.sol"; // mock class using ERC20 Token -contract ERC20TokenMock is ERC20 { +contract ERC20Mock is ERC20 { constructor(address initialAccount, uint256 initialBalance) public { _mint(initialAccount, initialBalance); diff --git a/contracts/mocks/ERC827Mock.sol b/contracts/mocks/ERC827Mock.sol new file mode 100644 index 0000000..a8866d8 --- /dev/null +++ b/contracts/mocks/ERC827Mock.sol @@ -0,0 +1,14 @@ +pragma solidity ^0.5.0; + + +import "../ERC827/ERC827.sol"; + + +// mock class using ERC827 Token +contract ERC827Mock is ERC827 { + + constructor(address initialAccount, uint256 initialBalance, bytes memory proxyBytecode) public ERC827(proxyBytecode) { + _mint(initialAccount, initialBalance); + } + +} diff --git a/contracts/mocks/ERC827Receiver.sol b/contracts/mocks/ERC827Receiver.sol new file mode 100644 index 0000000..45d9d30 --- /dev/null +++ b/contracts/mocks/ERC827Receiver.sol @@ -0,0 +1,47 @@ +pragma solidity ^0.5.0; + +import "../ERC827/ERC827.sol"; +import "../utils/Create2.sol"; + +contract ERC827Receiver { + + event Show(bytes32 b32, uint256 number, string text, uint256 value); + event TokensTransfered(address from, uint256 amount); + + function showMessage(bytes32 message, uint256 number, string memory text) + public payable returns (bool) + { + emit Show(message, number, text, msg.value); + return true; + } + + function fail() public { + revert("ERC827Receiver function failed"); + } + + function callContarct(address to, bytes memory data) public returns (bool) { + // solium-disable-next-line security/no-low-level-calls, no-unused-vars + (bool success, bytes memory _data) = to.call(data); + require(success, "ERC827Receiver callContarct function failed"); + return true; + } + + function receiveTokens(address sender, address token) public { + uint256 allowance = ERC827(token).allowance(sender, address(this)); + ERC827(token).transferFrom(sender, address(this), allowance); + emit TokensTransfered(sender, allowance); + } + + function receiveVerifiedTokens(address sender, ERC827 token) public { + address proxy = Create2.computeAddress( + address(token), + keccak256(abi.encodePacked(sender, address(this), token.nonces(sender))), + token.proxyBytecode() + ); + require(msg.sender == proxy, "ERC827Receiver: Sender invalid"); + uint256 allowance = ERC827(token).allowance(sender, address(this)); + ERC827(token).transferFrom(sender, address(this), allowance); + emit TokensTransfered(sender, allowance); + } + +} diff --git a/contracts/mocks/ERC827TokenMock.sol b/contracts/mocks/ERC827TokenMock.sol deleted file mode 100644 index 18817dc..0000000 --- a/contracts/mocks/ERC827TokenMock.sol +++ /dev/null @@ -1,14 +0,0 @@ -pragma solidity ^0.5.0; - - -import "../ERC827/ERC827.sol"; - - -// mock class using ERC827 Token -contract ERC827TokenMock is ERC827 { - - constructor(address initialAccount, uint256 initialBalance) public ERC827() { - _mint(initialAccount, initialBalance); - } - -} diff --git a/contracts/mocks/MessageHelper.sol b/contracts/mocks/MessageHelper.sol deleted file mode 100644 index 1b6170f..0000000 --- a/contracts/mocks/MessageHelper.sol +++ /dev/null @@ -1,39 +0,0 @@ -pragma solidity ^0.5.0; - - -contract MessageHelper { - - event Show(bytes32 b32, uint256 number, string text); - event Buy(bytes32 b32, uint256 number, string text, uint256 value); - - function showMessage(bytes32 message, uint256 number, string memory text) - public returns (bool) - { - emit Show(message, number, text); - return true; - } - - function buyMessage(bytes32 message, uint256 number, string memory text) - public payable returns (bool) - { - emit Buy( - message, - number, - text, - msg.value - ); - return true; - } - - function fail() public { - revert("MessageHelper fail function failed"); - } - - function callContarct(address to, bytes memory data) public returns (bool) { - // solium-disable-next-line security/no-low-level-calls, no-unused-vars - (bool success, bytes memory _data) = to.call(data); - require(success, "MessageHelper callContarct function failed"); - return true; - } - -} diff --git a/contracts/utils/Create2.sol b/contracts/utils/Create2.sol new file mode 100644 index 0000000..b3ede31 --- /dev/null +++ b/contracts/utils/Create2.sol @@ -0,0 +1,63 @@ +pragma solidity ^0.5.0; + +/** + * @title Create2 + * + * @dev Utility library that focus on the use of CREATE2 EVM opcode for + * contracts deployment. It also provides a function to precompute the address + * where the smart contracts with the specified salt and bytecode would be + * deployed. + */ +library Create2 { + + // Event triggered when a contract is deployed + event Create2Deployed(address addr, bytes32 salt); + + /** + * @dev Deploy contract with CREATE2 + * @param salt The salt used to the contract address computation + * @param code The bytecode of of the contract to be deployed + */ + function deploy(bytes32 salt, bytes memory code) internal returns (address) { + address addr = _deploy(salt, code); + emit Create2Deployed(addr, salt); + return addr; + } + + /** + * @dev Function to compute the address of a contract created with CREATE2. + * @param deployer the address of the contract that will deploy the contract + * @param salt The salt used to the contract address computation + * @param code The bytecode of the contract to be deployed + * @return the computed address of the smart contract. + */ + function computeAddress( + address deployer, bytes32 salt, bytes memory code + ) internal view returns (address) { + bytes32 codeHash = keccak256(code); + bytes32 _data = keccak256( + abi.encodePacked(bytes1(0xff), deployer, salt, codeHash) + ); + return address(bytes20(_data << 96)); + } + + /** + * @dev Internal function to deploy contract with CREATE2 + * @param _salt The salt used to the contract address computation + * @param _code The bytecode of the contract to be deployed + */ + function _deploy( + bytes32 _salt, bytes memory _code + ) private returns(address) { + address _addr; + // solhint-disable-next-line no-inline-assembly + assembly { + _addr := create2(0, add(_code, 0x20), mload(_code), _salt) + if iszero(extcodesize(_addr)) { + revert(0, 0) + } + } + return _addr; + } + +} From 69c8752239712acf320ad21394b652278efd95c1 Mon Sep 17 00:00:00 2001 From: AugustoL Date: Tue, 11 Jun 2019 12:59:10 -0300 Subject: [PATCH 2/3] fix(tuffle): Use constantinople evmVersion that supports create2 --- truffle-config.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/truffle-config.js b/truffle-config.js index 419454b..7f743a1 100644 --- a/truffle-config.js +++ b/truffle-config.js @@ -43,6 +43,10 @@ module.exports = { compilers: { solc: { version: '0.5.2', + settings: { + evmVersion: 'constantinople', + }, }, + } }; From 683a3faf060420479a444cb2aee67b3a4470ff37 Mon Sep 17 00:00:00 2001 From: AugustoL Date: Tue, 11 Jun 2019 13:00:00 -0300 Subject: [PATCH 3/3] test(tests): Update test to new dinamyc proxies and add ERC827Reciever example --- test/ERC827/ERC20Migration.js | 9 +- test/ERC827/ERC827Token.js | 180 +++++++----------------- test/examples/ClaimOtherTokensAttack.js | 9 +- test/examples/ReceiveTokens.js | 56 ++++++++ test/examples/VaultAttack.js | 7 +- 5 files changed, 124 insertions(+), 137 deletions(-) create mode 100644 test/examples/ReceiveTokens.js diff --git a/test/ERC827/ERC20Migration.js b/test/ERC827/ERC20Migration.js index f97d7c2..ccef9de 100644 --- a/test/ERC827/ERC20Migration.js +++ b/test/ERC827/ERC20Migration.js @@ -1,7 +1,8 @@ import EVMRevert from '../helpers/EVMRevert'; -var ERC20TokenMock = artifacts.require('ERC20TokenMock'); -var ERC827Migratable = artifacts.require('ERC827Migratable'); +const ERC20Mock = artifacts.require('ERC20Mock'); +const ERC827Migratable = artifacts.require('ERC827Migratable'); +const ERC827Proxy = artifacts.require('ERC827Proxy'); require('chai').use(require('chai-as-promised')).should(); const assert = require('chai').assert; @@ -10,8 +11,8 @@ contract('ERC827Migratable', function (accounts) { let erc20Token, erc827Token; beforeEach(async function () { - erc20Token = await ERC20TokenMock.new(accounts[1], 100); - erc827Token = await ERC827Migratable.new(erc20Token.address); + erc20Token = await ERC20Mock.new(accounts[1], 100); + erc827Token = await ERC827Migratable.new(erc20Token.address, ERC827Proxy.bytecode); await erc20Token.transfer(accounts[2], 10, { from: accounts[1] }); await erc20Token.transfer(accounts[3], 20, { from: accounts[1] }); await erc20Token.transfer(accounts[4], 30, { from: accounts[1] }); diff --git a/test/ERC827/ERC827Token.js b/test/ERC827/ERC827Token.js index 8ae0270..1938b13 100644 --- a/test/ERC827/ERC827Token.js +++ b/test/ERC827/ERC827Token.js @@ -1,19 +1,24 @@ import EVMRevert from '../helpers/EVMRevert'; -var Message = artifacts.require('MessageHelper'); -var ERC827TokenMock = artifacts.require('ERC827Mock'); +const ERC827Receiver = artifacts.require('ERC827Receiver'); +const ERC827Mock = artifacts.require('ERC827Mock'); +const ERC827Proxy = artifacts.require('ERC827Proxy'); require('chai').use(require('chai-as-promised')).should(); const assert = require('chai').assert; contract('ERC827 Token', function (accounts) { - let token, message; + let token, receiverContract, extraData, extraDataFail; beforeEach(async function () { - token = await ERC827TokenMock.new(accounts[0], 100); + token = await ERC827Mock.new(accounts[0], 100, ERC827Proxy.bytecode); token.web3Instance = new web3.eth.Contract(token.abi, token.address); - message = await Message.new(); - message.web3Instance = new web3.eth.Contract(message.abi, message.address); + receiverContract = await ERC827Receiver.new(); + receiverContract.web3Instance = new web3.eth.Contract(receiverContract.abi, receiverContract.address); + extraDataFail = receiverContract.web3Instance.methods.fail().encodeABI(); + extraData = receiverContract.web3Instance.methods.showMessage( + web3.utils.toHex(123456), 666, 'Transfer Done' + ).encodeABI(); }); it('should return the correct totalSupply after construction', async function () { @@ -22,18 +27,14 @@ contract('ERC827 Token', function (accounts) { }); it('should return the correct allowance amount after approval', async function () { - let token = await ERC827TokenMock.new(accounts[0], 100); await token.approve(accounts[1], 100); - let allowance = await token.allowance(accounts[0], accounts[1]); - assert.equal(allowance, 100); + assert.equal(await token.allowance(accounts[0], accounts[1]), 100); }); it('should return correct balances after transfer', async function () { await token.transfer(accounts[1], 100); - let balance0 = await token.balanceOf(accounts[0]); - assert.equal(balance0, 0); - let balance1 = await token.balanceOf(accounts[1]); - assert.equal(balance1, 100); + assert.equal(await token.balanceOf(accounts[0]), 0); + assert.equal(await token.balanceOf(accounts[1]), 100); }); it('should throw an error when trying to transfer more than balance', async function () { @@ -43,12 +44,9 @@ contract('ERC827 Token', function (accounts) { it('should return correct balances after transfering from another account', async function () { await token.approve(accounts[1], 100); await token.transferFrom(accounts[0], accounts[2], 100, { from: accounts[1] }); - let balance0 = await token.balanceOf(accounts[0]); - assert.equal(balance0, 0); - let balance1 = await token.balanceOf(accounts[2]); - assert.equal(balance1, 100); - let balance2 = await token.balanceOf(accounts[1]); - assert.equal(balance2, 0); + assert.equal(await token.balanceOf(accounts[0]), 0); + assert.equal(await token.balanceOf(accounts[2]), 100); + assert.equal(await token.balanceOf(accounts[1]), 0); }); it('should throw an error when trying to transfer more than allowed', async function () { @@ -85,210 +83,140 @@ contract('ERC827 Token', function (accounts) { it( 'should allow payment through transfer' , async function () { - const extraData = message.web3Instance.methods.buyMessage( - web3.utils.toHex(123456), 666, 'Transfer Done' - ).encodeABI(); - const transaction = await token.transferAndCall( - message.address, 100, extraData, { from: accounts[0], value: 1000 } + receiverContract.address, 100, extraData, { from: accounts[0], value: 1000 } ); - assert.equal(2, transaction.receipt.rawLogs.length); - - assert.equal(await token.balanceOf(message.address), 100); - assert.equal(await web3.eth.getBalance(message.address), 1000); + assert.equal(3, transaction.receipt.rawLogs.length); + assert.equal(await token.balanceOf(receiverContract.address), 100); + assert.equal(await web3.eth.getBalance(receiverContract.address), 1000); }); it( 'should allow payment through approve' , async function () { - const extraData = message.web3Instance.methods.buyMessage( - web3.utils.toHex(123456), 666, 'Transfer Done' - ).encodeABI(); - const transaction = await token.approveAndCall( - message.address, 100, extraData, { from: accounts[0], value: 1000 } + receiverContract.address, 100, extraData, { from: accounts[0], value: 1000 } ); - assert.equal(2, transaction.receipt.rawLogs.length); - - assert.equal(await token.allowance(accounts[0], message.address), 100); - assert.equal(await web3.eth.getBalance(message.address), 1000); + assert.equal(3, transaction.receipt.rawLogs.length); + assert.equal(await token.allowance(accounts[0], receiverContract.address), 100); + assert.equal(await web3.eth.getBalance(receiverContract.address), 1000); }); it( 'should allow payment through transferFrom' , async function () { - const extraData = message.web3Instance.methods.buyMessage( - web3.utils.toHex(123456), 666, 'Transfer Done' - ).encodeABI(); - await token.approve(accounts[1], 100, { from: accounts[0] }); - assert.equal(await token.allowance(accounts[0], accounts[1]), 100); const transaction = await token.transferFromAndCall( - accounts[0], message.address, 100, extraData, { from: accounts[1], value: 1000 } + accounts[0], receiverContract.address, 100, extraData, { from: accounts[1], value: 1000 } ); - assert.equal(2, transaction.receipt.logs.length); - - assert.equal( - await token.balanceOf(message.address), 100 - ); - assert.equal( - await web3.eth.getBalance(message.address), 1000 - ); + assert.equal(4, transaction.receipt.rawLogs.length); + assert.equal(await token.balanceOf(receiverContract.address), 100); + assert.equal(await web3.eth.getBalance(receiverContract.address), 1000); }); it('should revert funds of failure inside approve (with data)', async function () { - const extraData = message.web3Instance.methods.showMessage( - web3.utils.toHex(123456), 666, 'Transfer Done' - ).encodeABI(); - await token.approveAndCall( - message.address, 10, extraData, { from: accounts[0], value: 1000 } + receiverContract.address, 10, extraDataFail, { from: accounts[0], value: 1000 } ).should.be.rejectedWith(EVMRevert); // approval should not have gone through so allowance is still 0 - assert.equal(await token.allowance(accounts[1], message.address), 0); - assert.equal(await web3.eth.getBalance(message.address), 0); + assert.equal(await token.allowance(accounts[1], receiverContract.address), 0); + assert.equal(await web3.eth.getBalance(receiverContract.address), 0); }); it('should revert funds of failure inside transfer (with data)', async function () { - const extraData = message.web3Instance.methods.showMessage( - web3.utils.toHex(123456), 666, 'Transfer Done' - ).encodeABI(); - await token.transferAndCall( - message.address, 10, extraData, { from: accounts[0], value: 1000 } + receiverContract.address, 10, extraDataFail, { from: accounts[0], value: 1000 } ).should.be.rejectedWith(EVMRevert); // transfer should not have gone through, so balance is still 0 - assert.equal(await token.balanceOf(message.address), 0); - assert.equal(await web3.eth.getBalance(message.address), 0); + assert.equal(await token.balanceOf(receiverContract.address), 0); + assert.equal(await web3.eth.getBalance(receiverContract.address), 0); }); it('should revert funds of failure inside transferFrom (with data)', async function () { - const extraData = message.web3Instance.methods.showMessage( - web3.utils.toHex(123456), 666, 'Transfer Done' - ).encodeABI(); - await token.approve(accounts[1], 10, { from: accounts[2] }); - await token.transferFromAndCall( - accounts[2], message.address, 10, extraData, { from: accounts[2], value: 1000 } + accounts[2], receiverContract.address, 10, extraData, { from: accounts[2], value: 1000 } ).should.be.rejectedWith(EVMRevert); // transferFrom should have failed so balance is still 0 but allowance is 10 assert.equal(await token.allowance(accounts[2], accounts[1]), 10); - assert.equal(await token.balanceOf(message.address), 0); - assert.equal(await web3.eth.getBalance(message.address), 0); + assert.equal(await token.balanceOf(receiverContract.address), 0); + assert.equal(await web3.eth.getBalance(receiverContract.address), 0); }); it( 'should return correct balances after transfer (with data) and show the event on receiver contract' , async function () { - const extraData = message.web3Instance.methods.showMessage( - web3.utils.toHex(123456), 666, 'Transfer Done' - ).encodeABI(); - - const transaction = await token.transferAndCall(message.address, 100, extraData); - - assert.equal(2, transaction.receipt.rawLogs.length); - - assert.equal(await token.balanceOf(message.address), 100); + const transaction = await token.transferAndCall(receiverContract.address, 100, extraData); + assert.equal(3, transaction.receipt.rawLogs.length); + assert.equal(await token.balanceOf(receiverContract.address), 100); }); it( 'should return correct allowance after approve (with data) and show the event on receiver contract' , async function () { - const extraData = message.web3Instance.methods.showMessage( - web3.utils.toHex(123456), 666, 'Transfer Done' - ).encodeABI(); - - const transaction = await token.approveAndCall(message.address, 100, extraData); - - assert.equal(2, transaction.receipt.rawLogs.length); - - assert.equal(await token.allowance(accounts[0], message.address), 100); + const transaction = await token.approveAndCall(receiverContract.address, 100, extraData); + assert.equal(3, transaction.receipt.rawLogs.length); + assert.equal(await token.allowance(accounts[0], receiverContract.address), 100); }); it( 'should return correct balances after transferFrom (with data) and show the event on receiver contract' , async function () { - const extraData = message.web3Instance.methods.showMessage( - web3.utils.toHex(123456), 666, 'Transfer Done' - ).encodeABI(); - await token.approve(accounts[1], 100, { from: accounts[0] }); - assert.equal(await token.allowance(accounts[0], accounts[1]), 100); - - const transaction = await token.transferFromAndCall(accounts[0], message.address, 100, extraData, { + const transaction = await token.transferFromAndCall(accounts[0], receiverContract.address, 100, extraData, { from: accounts[1], }); - - assert.equal(2, transaction.receipt.logs.length); - - assert.equal(await token.balanceOf(message.address), 100); + assert.equal(4, transaction.receipt.rawLogs.length); + assert.equal(await token.balanceOf(receiverContract.address), 100); }); it('should fail inside approve (with data)', async function () { - const extraData = message.web3Instance.methods.fail().encodeABI(); - - await token.approveAndCall(message.address, 10, extraData) + await token.approveAndCall(receiverContract.address, 10, extraDataFail) .should.be.rejectedWith(EVMRevert); // approval should not have gone through so allowance is still 0 - assert.equal(await token.allowance(accounts[1], message.address), 0); + assert.equal(await token.allowance(accounts[1], receiverContract.address), 0); }); it('should fail inside transfer (with data)', async function () { - const extraData = message.web3Instance.methods.fail().encodeABI(); - - await token.transferAndCall(message.address, 10, extraData) + await token.transferAndCall(receiverContract.address, 10, extraDataFail) .should.be.rejectedWith(EVMRevert); // transfer should not have gone through, so balance is still 0 - assert.equal(await token.balanceOf(message.address), 0); + assert.equal(await token.balanceOf(receiverContract.address), 0); }); it('should fail inside transferFrom (with data)', async function () { - const extraData = message.web3Instance.methods.fail().encodeABI(); - await token.approve(accounts[1], 10, { from: accounts[2] }); - await token.transferFromAndCall(accounts[2], message.address, 10, extraData, { from: accounts[1] }) + await token.transferFromAndCall(accounts[2], receiverContract.address, 10, extraDataFail, { from: accounts[1] }) .should.be.rejectedWith(EVMRevert); // transferFrom should have failed so balance is still 0 but allowance is 10 assert.equal(await token.allowance(accounts[2], accounts[1]), 10); - assert.equal(await token.balanceOf(message.address), 0); + assert.equal(await token.balanceOf(receiverContract.address), 0); }); it('should fail approve (with data) when using token contract address as receiver', async function () { - const extraData = message.web3Instance.methods.fail().encodeABI(); - await token.approveAndCall(token.address, 100, extraData, { from: accounts[0] }) .should.be.rejectedWith(EVMRevert); }); it('should fail transfer (with data) when using token contract address as receiver', async function () { - const extraData = message.web3Instance.methods.showMessage( - web3.utils.toHex(123456), 666, 'Transfer Done' - ).encodeABI(); - - await token.transferAndCall(token.address, 100, extraData) + await token.transferAndCall(token.address, 100, extraData, { from: accounts[0] }) .should.be.rejectedWith(EVMRevert); }); it('should fail transferFrom (with data) when using token contract address as receiver', async function () { - const extraData = message.web3Instance.methods.showMessage( - web3.utils.toHex(123456), 666, 'Transfer Done' - ).encodeABI(); - await token.approve(accounts[1], 1, { from: accounts[0] }); - await token.transferFromAndCall(accounts[0], token.address, 1, extraData, { from: accounts[1] }) .should.be.rejectedWith(EVMRevert); }); diff --git a/test/examples/ClaimOtherTokensAttack.js b/test/examples/ClaimOtherTokensAttack.js index 12cacdd..3789fd4 100644 --- a/test/examples/ClaimOtherTokensAttack.js +++ b/test/examples/ClaimOtherTokensAttack.js @@ -1,7 +1,8 @@ import EVMRevert from '../helpers/EVMRevert'; -var ERC827TokenMock = artifacts.require('ERC827TokenMock'); -var ERC20TokenMock = artifacts.require('ERC20TokenMock'); +const ERC827Mock = artifacts.require('ERC827Mock'); +const ERC20Mock = artifacts.require('ERC20Mock'); +const ERC827Proxy = artifacts.require('ERC827Proxy'); require('chai') .use(require('chai-as-promised')) @@ -11,8 +12,8 @@ contract('ClaimOtherTokensAttack', function ([attacker, victim]) { let erc827, erc20; beforeEach(async function () { - erc827 = await ERC827TokenMock.new(attacker, 100); - erc20 = await ERC20TokenMock.new(victim, 100); + erc827 = await ERC827Mock.new(attacker, 100, ERC827Proxy.bytecode); + erc20 = await ERC20Mock.new(victim, 100); erc20.web3Instance = new web3.eth.Contract(erc20.abi, erc20.address); }); diff --git a/test/examples/ReceiveTokens.js b/test/examples/ReceiveTokens.js new file mode 100644 index 0000000..35104fa --- /dev/null +++ b/test/examples/ReceiveTokens.js @@ -0,0 +1,56 @@ + +import EVMRevert from '../helpers/EVMRevert'; +const ERC827Mock = artifacts.require('ERC827Mock'); +const ERC827Receiver = artifacts.require('ERC827Receiver'); +const ERC827Proxy = artifacts.require('ERC827Proxy'); + +require('chai') + .use(require('chai-as-promised')) + .should(); + +contract('ERC827Receiver', function ([sender, otherAccount]) { + let token, receiverContract; + + beforeEach(async function () { + token = await ERC827Mock.new(sender, 100, ERC827Proxy.bytecode); + receiverContract = await ERC827Receiver.new(); + receiverContract.web3Instance = new web3.eth.Contract(ERC827Receiver.abi, receiverContract.address); + }); + + it('should claim the tokens from sender', async function () { + const callReceiverData = receiverContract.web3Instance.methods + .receiveTokens(sender, token.address).encodeABI(); + await token.approveAndCall(receiverContract.address, 100, callReceiverData, { from: sender }); + + assert.equal(await token.balanceOf(sender), 0); + assert.equal(await token.balanceOf(receiverContract.address), 100); + }); + + it('should claim the tokens from the verified sender', async function () { + const callReceiverData = receiverContract.web3Instance.methods + .receiveVerifiedTokens(sender, token.address).encodeABI(); + await token.approveAndCall(receiverContract.address, 100, callReceiverData, { from: sender }); + + assert.equal(await token.balanceOf(sender), 0); + assert.equal(await token.balanceOf(receiverContract.address), 100); + }); + + it('should fail when executing verifiedTransfer from external accounts', async function () { + const callReceiverData = receiverContract.web3Instance.methods + .receiveVerifiedTokens(sender, token.address).encodeABI(); + await token.approve(receiverContract.address, 100, { from: sender }); + + // Trying to execute transferFrom on receiver contract over sender balance form other account + await receiverContract.receiveVerifiedTokens(sender, token.address, { from: otherAccount }) + .should.be.rejectedWith(EVMRevert); + + // Trying to execute transferFrom on receiver contract over sender balance form sender account + await receiverContract.receiveVerifiedTokens(sender, token.address, { from: sender }) + .should.be.rejectedWith(EVMRevert); + + + assert.equal(await token.balanceOf(sender), 100); + assert.equal(await token.balanceOf(otherAccount), 0); + assert.equal(await token.balanceOf(receiverContract.address), 0); + }); +}); diff --git a/test/examples/VaultAttack.js b/test/examples/VaultAttack.js index 703f562..ba037ac 100644 --- a/test/examples/VaultAttack.js +++ b/test/examples/VaultAttack.js @@ -1,7 +1,8 @@ import EVMRevert from '../helpers/EVMRevert'; -var ERC827TokenMock = artifacts.require('ERC827TokenMock'); -var VaultAttack = artifacts.require('VaultAttack'); +const ERC827Mock = artifacts.require('ERC827Mock'); +const VaultAttack = artifacts.require('VaultAttack'); +const ERC827Proxy = artifacts.require('ERC827Proxy'); require('chai') .use(require('chai-as-promised')) @@ -11,7 +12,7 @@ contract('VaultAttack', function ([victim, attacker]) { let token, vault; beforeEach(async function () { - token = await ERC827TokenMock.new(victim, 100); + token = await ERC827Mock.new(victim, 100, ERC827Proxy.bytecode); vault = await VaultAttack.new(); vault.web3Instance = new web3.eth.Contract(vault.abi, vault.address); });