theta token logo

Theta Token Sale Security Audit

Reading Time: 3 minutes

  •  
  •  
  •  
  •  
  •  
  •  

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:

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:

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:

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:

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

Do you want to know what is Coinfabrik Auditing Process?

Check our A-Z Smart Contract Audit Guide or you could request a quote for your project.


  •  
  •  
  •  
  •  
  •  
  •