The WAX team asked us to review and audit their WAX Token contract. We looked at the code and now publish our results.
The audited code is located in the waxio/wax-erc20-delivery-contract repository. The version used for this report is commit
Here is our assessment and recommendations, in order of importance.
Update: The WAX team has followed our recommendations and updated the WAX Token contract. The new version is at commit
No critical severity issues were found.
No high severity issues were found.
No medium severity issues were found.
Install OpenZeppelin via NPM
StandardToken were copied from the OpenZeppelin repository. Moreover, they appear to have been copied from the
master branch, instead of the release. This violates OpenZeppelin’s MIT license, which requires the license and copyright notice to be included if its code is used, and makes it difficult and error-prone to update to a more recent version.
Update: Impoted OpenZeppelin as NPM dependency in
OpenZeppelin standard contracts were modified
Additionally to copying OpenZeppelin’s contracts instead of installing them via NPM, some of them were modified.
BasicToken was modified to add a state variable
contractAddress, and the functions
StandardToken were modified to require that the receiver of tokens is not the
WaxToken contract itself.
This is not the way OpenZeppelin standard contracts should be used. Making changes to open-source libraries, instead of using them as is, can be dangerous and won’t allow you to integrate bug-fixes into the codebase easily.
Add these additional preconditions to the
WaxToken contract, instead of OpenZeppelin’s contracts. For example, override the
transfer function adding the additional restriction and call
super.transfer(…) within it. (Note: this has actually already been done, so it’s a matter of removing the modifications to OpenZeppelin’s contracts.)
Update: Fixed in
Token allowances may be increased or decreased when paused
Consider overriding in
decreaseApproval methods of the OpenZeppelin
StandardToken contract, adding the
Notice that this is already implemented in OpenZeppelin as
PausableToken. Consider using it instead.
Update: Fixed as suggested in
Notes & Additional Information
- The added fallback function with a manual
revertis not necessary, since this is the default behavior of Solidity contracts. Consider removing it.
WaxTokencontract is requiring that the receiver is not the zero address in
StandardTokenalready have this precondition. Consider removing those validations from the
- In the
WaxTokenconstructor, a reference to
this(the address of the contract) is stored in a variable called
contractAddress. We see no reason why this variable is used instead of directly using the value of
thisthroughout the contract.
- The functions
transferFromhave a precondition to reject a transfer of tokens to the
WaxTokencontract itself. It would be good to define a modifier that ensures this condition to avoid code duplication.
WaxTokencontract defines all its public functions without the
publickeyword explicitly. It’s important to mark
publicfunctions as such, even though Solidity functions are public by default, to avoid confusion. Moreover, since version 0.4.17 of Solidity, the compiler will show a warning if the visibility specifier (
public) is not explicitly given.
INITIAL_SUPPLYvariable is initialized multiplying
1000000000by the corresponding amount of decimals defined for the contract. Such a long number is hard to verify manually. It would be better to use the notation
1e9to represent this kind of number in order to avoid typos.
- Keep in mind that there is a possible attack vector on the
transferFromfunctionality of ERC20 tokens, described here. Consider using the mitigations implemented in OpenZeppelin’s
No critical or high severity issues were found. Some changes were proposed to follow best practices and reduce potential attack surface.
Note that as of the date of publishing, the above review reflects the current understanding of known security patterns as they relate to the WAX Token contracts. We have not reviewed the related WAX project. The above should not be construed as investment advice. For general information about smart contract security, check out our thoughts here.