Smart contract audit for Refactor project
Code repository: https://github.com/refactorteam/Crowdsale-Contracts
The version of the contracts for which the audit was conducted: https://github.com/refactorteam/Crowdsale-Contracts/tree/144b38652a5ed501b5d3f6e9256c9067bba71b1d
Revision version: https://github.com/refactorteam/Crowdsale-Contracts/compare/144b38652a5ed501b5d3f6e9256c9067bba71b1d…cedc5ff105e332b7fa1aa1e23600d4d91453a901
Classification of identified problems
CRITICAL – the possibility of theft of the ether / tokens or their blocking without the possibility of restoring access or other loss of ether / tokens due to any party, for example, dividends.
SERIOUS – the possibility of violations of the contract, in which to restore its correct operation, it is necessary to modify the state of the contract manually or completely replace it.
WARNINGS – the possibility of violation of the planned logic of the contract or the possibility of organizing a DoS attack on the contract.
NOTES – all other remarks.
Methodology of audit
The contract code is manually scanned for known vulnerabilities, logic errors, WhitePaper compliance. If necessary, unit tests are written for questionable moments.
Identified problems
– None
1. https://github.com/refactorteam/Crowdsale-Contracts/blob/144b38652a5ed501b5d3f6e9256c9067bba71b1d/REFACTOR%20TOKEN.sol#L197
The technical characteristics of the token indicate that the token contract must have a “burning” function. This function is implemented, but you can only burn your own tokens. Judging by the technical characteristics (the remainder of unsold tokens can be manually “burned” at the end of all activities) there should be a function of burning tokens by the organizers. Thus, in the current version of the contract, unsold tokens will remain on the crowdsale contract without the possibility of withdrawal.
[WARNINGS]
1. https://github.com/refactorteam/Crowdsale-Contracts/blob/144b38652a5ed501b5d3f6e9256c9067bba71b1d/REFACTOR%20TOKEN.sol#L108
In the version of the StandardToken contract from the Zeppelin Solidity library, an error error was found https://github.com/OpenZeppelin/zeppelin-solidity/issues/375, which allows to generate a false event about the transfer to itself of any number of tokens, even exceeding the ones available.
2. https://github.com/refactorteam/Crowdsale-Contracts/blob/144b38652a5ed501b5d3f6e9256c9067bba71b1d/REFACTOR%20TOKEN.sol#L283
Probably, in the formula the excess division by 1 ETH. with the current formula, to set the price of the token to 1 dollar, you need to call the setRate method with the parameter 300 * 10 ^ 18. It is better to implement the rate without such a number of zeros, so that there are fewer opportunities to make a mistake.
Implemented in https://github.com/refactorteam/Crowdsale-Contracts/commit/cedc5ff105e332b7fa1aa1e23600d4d91453a901
3. https://github.com/refactorteam/Crowdsale-Contracts/blob/144b38652a5ed501b5d3f6e9256c9067bba71b1d/REFACTOR%20TOKEN.sol#L202
We also recommend adding an event:
Transfer (burner, 0, _value);
in order for wallets, etherscan.io and other clients to see the fact of the balance change. Without this, many investors simply will not see their tokens in their wallets.
A similar problem exists in the token constructor,
https://github.com/refactorteam/Crowdsale-Contracts/blob/144b38652a5ed501b5d3f6e9256c9067bba71b1d/REFACTOR%20TOKEN.sol#L219 , there should be added:
Transfer(0, ADDRESS_FOR_TOKENS, INITIAL_SUPPLY);
[NOTES]
1. The latest version of the Zeppelin Solidity library is not used. Changes in the new version:
fixed a bug in the transferFrom function of the StandardToken contract (see p.1 of the warnings)
In the functions of contracts explicitly specified visibility modifiers
optimization in mul function from SafeMath, slightly reducing gas consumption
in the contracts BasicToken, StandardToken, BurnableToken in the functions transfer, transferFrom and burn are added checks of input parameters that do not burn all gas in case of failure: a check for transfer to a zero contract and that the value for transfer / incineration is less than the balance and allowable for transfer amount.
2. https://github.com/refactorteam/Crowdsale-Contracts/blob/144b38652a5ed501b5d3f6e9256c9067bba71b1d/REFACTOR%20TOKEN.sol#L215
The field digits in the token usually do the type uint8, this field is not described in ERC20, but it was suggested in ERC223 (https://github.com/ethereum/eips/issues/223). In the example on ethereum.org (https://ethereum.org/token) and in the Zeppelin Solidity library in the example (https://github.com/OpenZeppelin/zeppelin-solidity/pull/490) uint8
Implemented in https://github.com/refactorteam/Crowdsale-Contracts/commit/cedc5ff105e332b7fa1aa1e23600d4d91453a901
3. https://github.com/refactorteam/Crowdsale-Contracts/blob/144b38652a5ed501b5d3f6e9256c9067bba71b1d/REFACTOR%20TOKEN.sol#L209
The technical characteristics of the token indicate that the token contract must have such functions as “Transfer of owner rights” and “Are Ownable”. For a token contract, this is not done, but it may be necessary if the function of burning tokens in the token contract is added.
4. In the mechanics of the contract, there is a reference to the “shelf life” of the contract (which is 365 days). This is not implemented.
5. In the comments to those. the description is written, why the signature of three people of actions with contracts was not realized. Usually, to make these minuses irrelevant, they make it possible to perform actions that are not signed by all the owners. For example, signed by two owners of three.
6. In the comments to those. The description is written, why the distribution of funds for purses was not realized. But to find out the balance of any address is quite simple: http://solidity.readthedocs.io/en/develop/units-and-global-variables.html#address-related
7. https://github.com/refactorteam/Crowdsale-Contracts/blob/144b38652a5ed501b5d3f6e9256c9067bba71b1d/REFACTOR%20TOKEN.sol#L281
There is no check for the amount of transmitted ETH, you can buy 0 tokens.
Implemented in в https://github.com/refactorteam/Crowdsale-Contracts/commit/cedc5ff105e332b7fa1aa1e23600d4d91453a901
8. https://github.com/refactorteam/Crowdsale-Contracts/blob/144b38652a5ed501b5d3f6e9256c9067bba71b1d/REFACTOR%20TOKEN.sol#L221
https://github.com/refactorteam/Crowdsale-Contracts/blob/144b38652a5ed501b5d3f6e9256c9067bba71b1d/REFACTOR%20TOKEN.sol#L270
Blanks ADDRESS_FOR_TOKENS, ADDRESS_FOR_ETH, START_DATETIME – bad practice, because will require code modification before deployment, which is fraught with errors.
9. https://github.com/refactorteam/Crowdsale-Contracts/blob/144b38652a5ed501b5d3f6e9256c9067bba71b1d/REFACTOR%20TOKEN.sol#L236
modifier onlyOwner is redundant inCrowdsale
10. https://github.com/refactorteam/Crowdsale-Contracts/blob/144b38652a5ed501b5d3f6e9256c9067bba71b1d/REFACTOR%20TOKEN.sol#L271
Pointless line. In addition, the default rate is 0 and its installation is not monitored before / during the beginning of crowdsale – this is fraught with the fact that if you forget or do not have time to install it, payments will pass for which 0 tokens will be credited.
It is necessary to specify in the instruction about the need to set the rate in this line before the contract is deployed and allocated visually.
Implemented in https://github.com/refactorteam/Crowdsale-Contracts/commit/cedc5ff105e332b7fa1aa1e23600d4d91453a901