From 209e2de93be06110bc7133b46f25a773d0a4c427 Mon Sep 17 00:00:00 2001 From: Pavel Rubin Date: Mon, 28 Aug 2017 21:00:29 +0300 Subject: [PATCH 1/4] Fix for #400: Check that destination of token transfers is not 0x --- contracts/token/BasicToken.sol | 2 ++ contracts/token/StandardToken.sol | 2 ++ 2 files changed, 4 insertions(+) diff --git a/contracts/token/BasicToken.sol b/contracts/token/BasicToken.sol index ef35a37a8..e584df274 100644 --- a/contracts/token/BasicToken.sol +++ b/contracts/token/BasicToken.sol @@ -20,6 +20,8 @@ contract BasicToken is ERC20Basic { * @param _value The amount to be transferred. */ function transfer(address _to, uint256 _value) returns (bool) { + require(_to != address(0)); + balances[msg.sender] = balances[msg.sender].sub(_value); balances[_to] = balances[_to].add(_value); Transfer(msg.sender, _to, _value); diff --git a/contracts/token/StandardToken.sol b/contracts/token/StandardToken.sol index 358d6f05e..74b16a65a 100644 --- a/contracts/token/StandardToken.sol +++ b/contracts/token/StandardToken.sol @@ -24,6 +24,8 @@ contract StandardToken is ERC20, BasicToken { * @param _value uint256 the amount of tokens to be transferred */ function transferFrom(address _from, address _to, uint256 _value) returns (bool) { + require(_to != address(0)); + var _allowance = allowed[_from][msg.sender]; // Check is not needed because sub(_allowance, _value) will already throw if this condition is not met From d095ba84bf8fe6f2ed1620ca8ee809d2dcf21c7b Mon Sep 17 00:00:00 2001 From: Pavel Rubin Date: Tue, 29 Aug 2017 00:10:40 +0300 Subject: [PATCH 2/4] Add tests to check transfers to 0x0 fail --- test/BasicToken.js | 10 ++++++++++ test/StandardToken.js | 10 ++++++++++ 2 files changed, 20 insertions(+) diff --git a/test/BasicToken.js b/test/BasicToken.js index ee9bc9079..76248948a 100644 --- a/test/BasicToken.js +++ b/test/BasicToken.js @@ -32,4 +32,14 @@ contract('BasicToken', function(accounts) { } }); + it("should throw an error when trying to transfer to 0x0", async function() { + let token = await StandardTokenMock.new(accounts[0], 100); + try { + let transfer = await token.transfer(0x0, 100); + assert.fail('should have thrown before'); + } catch(error) { + assertJump(error); + } + }); + }); diff --git a/test/StandardToken.js b/test/StandardToken.js index 96a51afe5..acf53c31e 100644 --- a/test/StandardToken.js +++ b/test/StandardToken.js @@ -88,4 +88,14 @@ contract('StandardToken', function(accounts) { }) }); + it("should throw an error when trying to transfer to 0x0", async function() { + let token = await StandardTokenMock.new(accounts[0], 100); + try { + let transfer = await token.transfer(0x0, 100); + assert.fail('should have thrown before'); + } catch(error) { + assertJump(error); + } + }); + }); From 74db6c2b3b2d52705b034b713c5efb5dbaced570 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Mon, 28 Aug 2017 19:17:49 -0300 Subject: [PATCH 3/4] add missing test for error when doing transferFrom to 0x0 --- test/BasicToken.js | 6 +++--- test/StandardToken.js | 15 +++++++++++++-- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/test/BasicToken.js b/test/BasicToken.js index 76248948a..e5bb46868 100644 --- a/test/BasicToken.js +++ b/test/BasicToken.js @@ -22,7 +22,7 @@ contract('BasicToken', function(accounts) { assert.equal(secondAccountBalance, 100); }); - it("should throw an error when trying to transfer more than balance", async function() { + it('should throw an error when trying to transfer more than balance', async function() { let token = await BasicTokenMock.new(accounts[0], 100); try { let transfer = await token.transfer(accounts[1], 101); @@ -32,14 +32,14 @@ contract('BasicToken', function(accounts) { } }); - it("should throw an error when trying to transfer to 0x0", async function() { + it('should throw an error when trying to transfer to 0x0', async function() { let token = await StandardTokenMock.new(accounts[0], 100); try { let transfer = await token.transfer(0x0, 100); assert.fail('should have thrown before'); } catch(error) { assertJump(error); - } + } }); }); diff --git a/test/StandardToken.js b/test/StandardToken.js index acf53c31e..cb4e9b038 100644 --- a/test/StandardToken.js +++ b/test/StandardToken.js @@ -88,14 +88,25 @@ contract('StandardToken', function(accounts) { }) }); - it("should throw an error when trying to transfer to 0x0", async function() { + it('should throw an error when trying to transfer to 0x0', async function() { let token = await StandardTokenMock.new(accounts[0], 100); try { let transfer = await token.transfer(0x0, 100); assert.fail('should have thrown before'); } catch(error) { assertJump(error); - } + } + }); + + it('should throw an error when trying to transferFrom to 0x0', async function() { + let token = await StandardTokenMock.new(accounts[0], 100); + await token.approve(accounts[1], 100); + try { + let transfer = await token.transferFrom(accounts[0], 0x0, 100, {from: accounts[1]}); + assert.fail('should have thrown before'); + } catch(error) { + assertJump(error); + } }); }); From 00f80c726abbddee5bafc5936985ddd259b447eb Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Mon, 28 Aug 2017 19:29:15 -0300 Subject: [PATCH 4/4] fix reference to mock contract --- test/BasicToken.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/BasicToken.js b/test/BasicToken.js index e5bb46868..261a68c3b 100644 --- a/test/BasicToken.js +++ b/test/BasicToken.js @@ -33,7 +33,7 @@ contract('BasicToken', function(accounts) { }); it('should throw an error when trying to transfer to 0x0', async function() { - let token = await StandardTokenMock.new(accounts[0], 100); + let token = await BasicTokenMock.new(accounts[0], 100); try { let transfer = await token.transfer(0x0, 100); assert.fail('should have thrown before');