This document is a security audit report performed by RideSolo, where CRAD CASH has been reviewed.
Token desription:
Symbol : CRAD
Name : CRAD CASH
Total supply: 100,000,000
Decimals : 18
Standard : ERC20/ERC223
6 issues were reported including:
-
1 high severity issue.
-
2 medium severity issue.
-
3 low severity issues.
Implementing both ERC20 & ERC223 in the same contract like it is done in Crad Cash token does not make sense since the implementation still allow token transfer to contracts that may handle token transfers using transfer(address _to, uint256 _value)
, when in a normal ERC223 implementation transfer(address _to, uint256 _value)
will still call transfer(address _to, uint _value, bytes memory _data)
by just adding and empty _data
array.
This implementation does not prevent contracts and dapps or users to transfer tokens to contract since the most used function is transfer(address _to, uint256 _value)
and not transfer(address _to, uint256 _value, bytes memory _data)
. all related issues with ERC20 that ERC223 solves are still applicable.
https://gist.github.com/RideSolo/ab90f1c3a8808fd1ab7f286d4152fc76#file-crad-sol-L94
https://gist.github.com/RideSolo/ab90f1c3a8808fd1ab7f286d4152fc76#file-crad-sol-L146
When calling transfer(address _to, uint _value, bytes memory _data)
function, if _to
address is a contract tokenFallback
is called before assigning the tokens to the contract balance, which will cause compatibiity issues since the ERC223 standard call tokenFallback
after assigning the tokens to the contract address, check here for more details.
https://gist.github.com/RideSolo/ab90f1c3a8808fd1ab7f286d4152fc76#file-crad-sol-L146
A transfer event should be triggered when initializing owner balance.
https://gist.github.com/RideSolo/ab90f1c3a8808fd1ab7f286d4152fc76#file-crad-sol-L175#L179
Any ether that is sent through the fallback function to the contrat is forwarded to the contract owner, developers should explain such logic.
https://gist.github.com/RideSolo/ab90f1c3a8808fd1ab7f286d4152fc76#file-crad-sol-L180
transfer(address _to, uint _value, bytes memory _data)
does not prevent from sending tokens to 0x0 address.
https://gist.github.com/RideSolo/ab90f1c3a8808fd1ab7f286d4152fc76#file-crad-sol-L146
Add a requirements to check if _to
address is different then address(0)
.
- It is possible to double withdrawal attack. More details here
- Lack of transaction handling mechanism issue. WARNING! This is a very common issue and it already caused millions of dollars losses for lots of token users! More details here
Add the following code to the transfer(_to address, ...)
function:
require( _to != address(this) );
All highlighted issues should be fixed. if the contract is intended to be used as ERC223 then the contract is not safe to be deployed.