One of our clients asked us to review and audit their token project. We have found one critical bug leading to collecting invalid amount of Ether during ICO sale – and you might like to take a closer look at this to avoid it in your own project.
Explaining the Smart Contract bug
Before we start, have a look at the below fragment of Tge.sol contract, specifically the initStates() function, where etherCap is set:
contract Tge is Minter { using SafeMath for uint; // state enum State {Presale, Preico1, Preico2, Break, Ico1, Ico2, Ico3, Ico4, Ico5, Ico6, FinishingIco, Allocating, Airdropping, Finished} State public currentState = State.Presale; mapping(uint => uint) public startTimes; mapping(uint => uint) public etherCaps; function Tge( CrowdfundableToken _token, uint _saleEtherCap, uint saleStartTime, uint singleStateEtherCap ) public Minter(_token, _saleEtherCap) { require(saleStartTime >= now); require(singleStateEtherCap > 0); initStates(saleStartTime, singleStateEtherCap); } // initialize states start times and caps function initStates(uint saleStart, uint singleStateEtherCap) internal { startTimes[uint(State.Preico1)] = saleStart; setStateLength(State.Preico1, 5 days); setStateLength(State.Preico2, 5 days); setStateLength(State.Break, 3 days); setStateLength(State.Ico1, 5 days); setStateLength(State.Ico2, 5 days); setStateLength(State.Ico3, 5 days); setStateLength(State.Ico4, 5 days); setStateLength(State.Ico5, 5 days); setStateLength(State.Ico6, 5 days); // the total sale ether cap is distributed evenly // over all selling states setEtherCap(State.Preico1, singleStateEtherCap); setEtherCap(State.Preico2, singleStateEtherCap); setEtherCap(State.Ico1, singleStateEtherCap); setEtherCap(State.Ico2, singleStateEtherCap); setEtherCap(State.Ico3, singleStateEtherCap); setEtherCap(State.Ico4, singleStateEtherCap); setEtherCap(State.Ico5, singleStateEtherCap); setEtherCap(State.Ico6, singleStateEtherCap); } function moveState(uint from, uint to) external onlyInUpdatedState onlyOwner { require(uint(currentState) == from); advanceStateIfNewer(State(to)); } function advanceStateIfNewer(State newState) internal { if (uint(newState) > uint(currentState)) { emit StateChanged(uint(currentState), uint(newState)); currentState = newState; } } function setStateLength(State state, uint length) internal { // state length is determined by next state's start time startTimes[uint(state)+1] = startTimes[uint(state)] .add(length); } function setEtherCap(State state, uint cap) internal { // accumulate cap from previous states if(state == State.Ico1) { etherCaps[uint(State.Ico1)] = etherCaps[uint(State.Ico1) - 2].add(cap); } else { etherCaps[uint(state)] = etherCaps[uint(state)-1] .add(cap); } } }
Consider the following code snippet from Tge.initStates():
... setEtherCap(State.Preico1, singleStateEtherCap); setEtherCap(State.Preico2, singleStateEtherCap); setEtherCap(State.Ico1, singleStateEtherCap); setEtherCap(State.Ico2, singleStateEtherCap); ...
The etherCaps mapping field should store accumulative ether caps for specific ICO states:
etherCaps = [0, 10, 20, 30, 40 ... 90]
It means 10 ether should be collected up to State.PreIco1, 20 Ether in total should be collected up to the next state and so on.
For the State.Break, which is the fourth value, the Tge contract stores 0, so the array looks like:
[0, 10, 20, 0, 10, 20, 30...]
This is a bug, since starting from the fifth value (10), the values should be hig