Skip to content

Instantly share code, notes, and snippets.

@yuriy77k
Forked from RideSolo/ETH_CradCash_report.md
Created April 27, 2019 15:33
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save yuriy77k/7cf31491fed613a2957689eac67abb6a to your computer and use it in GitHub Desktop.
Save yuriy77k/7cf31491fed613a2957689eac67abb6a to your computer and use it in GitHub Desktop.

CRAD CASH Audit Report.

1. Summary

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

2. In scope

3. Findings

6 issues were reported including:

  • 1 high severity issue.

  • 2 medium severity issue.

  • 3 low severity issues.

3.1. ERC223 & ERC20 Implementation

Severity: medium

Description

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.

Code snippet

https://gist.github.com/RideSolo/ab90f1c3a8808fd1ab7f286d4152fc76#file-crad-sol-L94

https://gist.github.com/RideSolo/ab90f1c3a8808fd1ab7f286d4152fc76#file-crad-sol-L146

3.2. ERC223 Transfer

Severity: High

Description

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.

Code snippet

https://gist.github.com/RideSolo/ab90f1c3a8808fd1ab7f286d4152fc76#file-crad-sol-L146

3.3. Transfer Event

Severity: low

Description

A transfer event should be triggered when initializing owner balance.

Code snippet

https://gist.github.com/RideSolo/ab90f1c3a8808fd1ab7f286d4152fc76#file-crad-sol-L175#L179

3.4. Fallback Function

Severity: medium

Description

Any ether that is sent through the fallback function to the contrat is forwarded to the contract owner, developers should explain such logic.

Code snippet

https://gist.github.com/RideSolo/ab90f1c3a8808fd1ab7f286d4152fc76#file-crad-sol-L180

3.4. Transfer to 0x0 Address

Severity: low

Description

transfer(address _to, uint _value, bytes memory _data) does not prevent from sending tokens to 0x0 address.

Code snippet

https://gist.github.com/RideSolo/ab90f1c3a8808fd1ab7f286d4152fc76#file-crad-sol-L146

Recommendation

Add a requirements to check if _to address is different then address(0).

3.6. Known vulnerabilities of ERC-20 token

Severity: low

Description

  1. It is possible to double withdrawal attack. More details here
  2. 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

Recommendation

Add the following code to the transfer(_to address, ...) function:

require( _to != address(this) );

4. Conclusion

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment