The Blockchain Capital Team asked us to review and audit their new BCAP Token contract code. We looked at their code and now publish our results.
The audited contract is at their BCAPToken GitHub repo. The version used for this report is commit 5cb5e76338cc47343ba9268663a915337c8b268e, corresponding to version 0.7. The main contract file is BCAPToken.sol.
Code quality is very good, and well commented and modularized. This is one of the best written projects we audited so far.
Update: the Blockchain Capital team fixed some of our recommendations in their latest version.
Here’s our assessment and recommendations, in order of importance:
We haven’t found any severe security problems with the code.
Current code is written for old versions of solc (0.4.1). With the storage overwriting vulnerability found on versions of solc prior to 0.4.4, there’s a risk this code can be compiled with vulnerable versions. We recommend changing the solidity version pragma for the latest version (pragma solidity ^0.4.10;) to enforce latest compiler version to be used.
EDIT: The Argon Group replied: “The pragma directive assures that no language constructions introduced after 0.4.1 are used — for the purpose of language maturity. It also allows compiling by a version up to 0.5.0 (not yet released). The current binaries are compiled with version 0.4.10. Therefore there is no risk of using a too old compiler version for this particular contract.”
The tokenIssuer address state variable in line 49 of StandardToken has full control of the token supply. Furthermore, it’s the same address that can freeze and unfreeze transactions in BCAPToken. The corresponding private key should have really high security standards. Consider using a multisig wallet for that address, and a key escrow service to further protect it.
Formal security audits are not enough to be safe. We recommend implementing an automated contract-based bug bounty and setting a period of time where security researchers from around the globe can try to break the contract’s invariants. For more info on how to implement automated bug bounties with OpenZeppelin, see this guide.
Duplicate code makes it harder to understand the code’s intention and thus, auditing the code correctly. It also increases the risk of introducing hidden bugs when modifying one of the copies of some code and not the others.
The logic of the contracts Token, AbstractToken, and SafeMath are very similar to OpenZeppelin library contracts ERC20Token, StandardToken, and SafeMath, respectively. Consider avoiding code repetition, which can bring regression problems and introduce unexpected bugs. Consider using the standard modules from OpenZeppelin.
Another instance of duplicated code is the owner variable of BCAPToken.sol. Consider using OpenZeppelin’s Ownable as an equivalent. Checks for owner being the msg.sender are implemented in Ownable too, but could also be extracted as a function modifier.
A simple yet powerful programming good practice is to make your code fail as promptly as possible. And be loud about it. We want to avoid a contract failing silently, or continuing execution in an unstable or inconsistent state. Consider changing all precondition checks to throwing to follow this good practice. Some parts of the code do this correctly, but we’d like to see more consistency on this pattern.
Some places where this could be improved are:
centralBank account, which has the power to create tokens at will. This doesn’t mean the ERC20 is not respected, but may be unexpected by some users.centralBank account.No severe security issues were found. Some changes were recommended to follow best practices and reduce potential attack surface.
Update: the Blockchain Capital team fixed some of our recommendations in their latest version.
Code quality is very good, and well commented and modularized. This is one of the best written projects we audited so far.
Note that as of the date of publishing, the above review reflects the current understanding of known security patterns as they relate to the BCAP Token contract. We have not reviewed the related Blockchain Capital project. The above should not be construed as investment advice or an offering of tokens. For general information about smart contract security, check out our thoughts here.