Theta Token Sale Security Audit

Coinfabrik was asked to audit the contracts for the Theta Token sale. In the first part, we will give a summary of our discoveries and follow them with the details of our findings.

The contracts audited are from the Theta repository at https://github.com/thetatoken/theta-erc20-token-sale. The audit is based on the commit 46592ac461949fa793b9d0dc1f59df9bf7ea07f3 and updated to reflect changes up until commit 3158c10d1a0b6347b9490aad68caab7e5f17d753.

Summary

The audited contracts are:

• SafeMath.sol: Safe math operations library
• StandardToken.sol: Inheritable class for ERC20 token implementation.
• ThetaToken.sol: The token contract itself, inherits StandardToken.
• ThetaTokenSale.sol: The token sale contract

Detailed findings

Medium severity

Bad handling of whitelist array

There is a whitelist array for addresses that is used to keep track of accounts that are able to buy tokens. This array is modified by a function to add accounts and another one to remove them:

// @notice Add accounts to the white list. Only whitelisted accounts can buy Theta tokens

function addAccountsToWhitelist(address[] _accounts) only(whitelistController) public {


     for (uint i = 0; i < _accounts.length; i ++) {


     address account = _accounts[i];
     if (whitelistMap[account]) {


     continue;
}


     whitelist.push(account);
     whitelistMap[account] = true;

}

}


function deleteAccountsFromWhitelist(address[] _accounts) only(whitelistController) public {

     for (uint i = 0; i < _accounts.length; i ++) {


     address account = _accounts[i];
     whitelistMap[account] = false;
     for (uint j = 0; j < whitelist.length; j ++) {


     if (account == whitelist[j]) {
     delete whitelist[j];
}

}

}

}

There are two potential problems here, the first one is the gas costs, since the function deleteAccountsFromWhitelist may fail if the array grows too much, which may be the case given that whitelisting is required for buying tokens. The second, which complements the first, is the delete in said function. This delete will zero the entry which means removing addresses will not reduce the size of the array, and since addresses are just pushed back when adding them depending on how these functions are used externally it may grow unexpectedly large, in the worst case softlocking the second function because of gas costs.

We recommend removing the array, leaving only the mapping, and keeping track of the account list externally instead. In the end, all whitelisting transactions will be saved in the blockchain so everyone can reconstruct the array if needed. You may also add an event to facilitate this.

This was fixed in commit 64b3d75b6431d75ebeb426ce935009d2187830c1, including the addition of the Whitelist event.

Minor severity

Missing minting call according to comments in finalizeSale

According to the comments in function finalizeSale, there should be a minting call here:

// @notice Finalizes sale generating the tokens for Theta Dev.

// @dev Transfers the token controller power to 0x00.

     function finalizeSale()
     only_after_sale
     only(root)
     public {


// This function cannot be successfully called twice, because it will set the controller to zero,
// and the mint call will fail if called again.


// Sale yields token controller to address 0x00

     token.changeController(0);

     saleFinalized = true;
     saleStopped = true;

}

If this is not the case, we recommend removing the comments as they are misleading.

This was acknowledged by the development team and the comments were removed at commit 64b3d75b6431d75ebeb426ce935009d2187830c1.

Enhancements

Unnecessary requirements in mint function at token

The first three statements in the mint function are unnecessary as both requires are already checked by using SafeMath functions:

function mint(address _owner, uint _amount) external only_controller returns (bool) {


     require(totalSupply + _amount >= totalSupply);

     uint previousBalance = balances[_owner];
     require(previousBalance + _amount >= previousBalance);

     totalSupply = totalSupply.add(_amount);
     balances[_owner] = balances[_owner].add(_amount);

     Transfer(0, _owner, _amount);
     return true;

}

Consider removing these lines to simplify the contract.

This was enhanced in commit 64b3d75b6431d75ebeb426ce935009d2187830c1.

Commented code in doPayment function

Remove commented code if it’s not going to be present in the final contract:

// @notice `doPayment()` is an internal function that sends the received ether

// to the fund deposit address, and mints tokens for the purchaser

     function doPayment(address _owner)
     only_sale_activated
     only_during_sale_period
     only_sale_not_stopped
     non_zero_address(_owner)
     at_least(dust)
     internal {
     uint fundReceived = msg.value;


// require(fundReceived <= fundCollectedHardCap.sub(fundCollected));

     require(fundCollected <= fundCollectedHardCap);


// Calculate how many tokens bought

     uint boughtTokens = msg.value.mul(exchangeRate);


// If past hard cap, throw

     uint tokenSoldAmount = token.totalSupply().mul(40).div(100); // 40% available for purchase


// require((tokenSoldAmount <= tokenSaleHardCap) && (boughtTokens <= tokenSaleHardCap.sub(tokenSoldAmount)));

     require((tokenSoldAmount <= tokenSaleHardCap));
     require(whitelistMap[_owner]);


// Send funds to fundDeposit

     require(fundDeposit.send(fundReceived));


// Allocate tokens. This will fail after sale is finalized in case it is hidden cap finalized.

     uint reserveTokens = calcReserve(boughtTokens);
     require(token.mint(thetaLabsReserve, reserveTokens));
     require(token.mint(_owner, boughtTokens));

// Save total collected amount

     fundCollected = fundCollected.add(msg.value);

}

This was enhanced in commit 64b3d75b6431d75ebeb426ce935009d2187830c1.

Conclusion

We found the contracts to be straightforward and simple, which is always the best thing to see in audits. We were pleasantly impressed by the contracts Code quality. Most functions have a few lines, which is another plus.

References

• ERC20 Token Standard, https://github.com/ethereum/EIPs/blob/master/EIPS/eip-20-token-standard.md