Smart Contract Audit – Critical Bug with Code Example

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