diff --git a/contracts/token/BasicToken.sol b/contracts/token/BasicToken.sol index 7df2500134d..a45157f81f8 100644 --- a/contracts/token/BasicToken.sol +++ b/contracts/token/BasicToken.sol @@ -21,6 +21,7 @@ contract BasicToken is ERC20Basic { */ function transfer(address _to, uint256 _value) public returns (bool) { require(_to != address(0)); + require(_value <= balances[msg.sender]); // SafeMath.sub will throw if there is not enough balance. balances[msg.sender] = balances[msg.sender].sub(_value); diff --git a/contracts/token/BurnableToken.sol b/contracts/token/BurnableToken.sol index 67cb850fe8f..c155bd1883a 100644 --- a/contracts/token/BurnableToken.sol +++ b/contracts/token/BurnableToken.sol @@ -16,6 +16,9 @@ contract BurnableToken is StandardToken { */ function burn(uint256 _value) public { require(_value > 0); + require(_value <= balances[msg.sender]); + // no need to require value <= totalSupply, since that would imply the + // sender's balance is greater than the totalSupply, which *should* be an assertion failure address burner = msg.sender; balances[burner] = balances[burner].sub(_value); diff --git a/contracts/token/StandardToken.sol b/contracts/token/StandardToken.sol index 061c4c958b7..7ab917c9571 100644 --- a/contracts/token/StandardToken.sol +++ b/contracts/token/StandardToken.sol @@ -25,15 +25,12 @@ contract StandardToken is ERC20, BasicToken { */ function transferFrom(address _from, address _to, uint256 _value) public returns (bool) { require(_to != address(0)); - - uint256 _allowance = allowed[_from][msg.sender]; - - // Check is not needed because sub(_allowance, _value) will already throw if this condition is not met - // require (_value <= _allowance); + require(_value <= balances[_from]); + require(_value <= allowed[_from][msg.sender]); balances[_from] = balances[_from].sub(_value); balances[_to] = balances[_to].add(_value); - allowed[_from][msg.sender] = _allowance.sub(_value); + allowed[_from][msg.sender] = allowed[_from][msg.sender].sub(_value); Transfer(_from, _to, _value); return true; } diff --git a/test/StandardToken.js b/test/StandardToken.js index 7dd11a782db..f82907ce007 100644 --- a/test/StandardToken.js +++ b/test/StandardToken.js @@ -1,6 +1,7 @@ 'use strict'; const assertJump = require('./helpers/assertJump'); +const expectThrow = require('./helpers/expectThrow'); var StandardTokenMock = artifacts.require('./helpers/StandardTokenMock.sol'); contract('StandardToken', function(accounts) { @@ -70,6 +71,17 @@ contract('StandardToken', function(accounts) { } }); + it('should throw an error when trying to transferFrom more than _from has', async function() { + let balance0 = await token.balanceOf(accounts[0]); + await token.approve(accounts[1], 99); + try { + await token.transferFrom(accounts[0], accounts[2], balance0+1, {from: accounts[1]}); + assert.fail('should have thrown before'); + } catch (error) { + assertJump(error); + } + }); + describe('validating allowance updates to spender', function() { let preApproved; @@ -88,6 +100,13 @@ contract('StandardToken', function(accounts) { }) }); + it('should increase by 50 then set to 0 when decreasing by more than 50', async function() { + await token.approve(accounts[1], 50); + await token.decreaseApproval(accounts[1], 60); + let postDecrease = await token.allowance(accounts[0], accounts[1]); + postDecrease.should.be.bignumber.equal(0); +}); + it('should throw an error when trying to transfer to 0x0', async function() { let token = await StandardTokenMock.new(accounts[0], 100); try {