Here is the report of the Natmin (NAT) Security Audit performed by the Callisto Network security department in September 2018.
About Callisto Network and the security department
Utilizing Callisto Network capabilities, we have established a free-for-all system of smart-contracts auditing, to this end, Callisto Network has founded the Callisto security department and deploys treasury funds to pay security auditors for auditing smart-contracts, to reduce risk/flaw in smart-contracts and improve the adoption of programmable blockchains for the whole crypto industry.
Natmin (NAT) specificities
Natmin Security Audit Report
The contract is safe to be deployed. However the current implementation of Natmin risks to conflict with contract when transferring token to them and throw the transfer transaction.
2. High severity issues
No High severity issues
3. Medium severity issues
3.1. ERC223 Standard Compliance
The reviewed token contract is not ERC223 fully compliant,
transferToContract function member of
NatminToken contract call
tokenFallback external function on the receiver contract before adding the
balances[_to]. The original implementation adds the token value to the balance before making the external call (check the link below).
Different issues can be raised depending on
tokenFallBack implementation on the receiver contract. A good example is if the contract tries to move the tokens from its balance when the tokens are not yet added to it.
3.2. Token Transfer to Address 0x0
The implemented version of ERC20/ERC223 Token do not require the
_to address in
transferFrom to be non null before token transfer. Accidental token loss to address 0x0 can be applicable.
4. Low severity issues
4.1. Vesting Logic
If the owner set a vesting amount and an end time for a user using
addVesting function, and if the user receives tokens from another address, he won’t be able to transfer the extra amount even if
balancesOf[user] > vestings[user].amount.
Actually purpose of
vestings[user].amount is unclear. It used only in function
getVestedAmount, but this function is unused in contract.
4.2. Known Issue of ERC20 Standard
This is just a reminder for the contract developers (the described ERC20 issue is well-known and well documented).
Approve + transferFrom mechanism allows double Withdrawal attack.
5. Minor observation
5.1. No need of require
SafeMath.sub() will automatically throw, if someone will try send more, than he has. In transfer and transferFrom functions no need to check it with require.
In lines 189, 208, 222 no need of require.