EthereumCommonwealth / Auditing

Ethereum Commonwealth Security Department conducted over 400 security audits since 2018. Not even a single contract that we audited was hacked. You can access our audit reports in the ISSUES of this repo. We are accepting new audit requests.
https://audits.callisto.network/
GNU General Public License v3.0
131 stars 34 forks source link

ZRX (ZRX) Token #211

Closed yuriy77k closed 5 years ago

yuriy77k commented 5 years ago

Audit request

Audit Top 200 CoinMarketCap tokens.

0x (ZRX)

https://0x.org/

Deployed at https://etherscan.io/address/0xe41d2489571d322189246dafa5ebde1f4699f498#code

Source code

https://gist.github.com/yuriy77k/9e3f16ce8289f6fafa64c9fee13dfd1f

Disclosure policy

Public

Platform

ETH

Number of lines:

77

MrCrambo commented 5 years ago

Auditing time 1 day.

yuriy77k commented 5 years ago

@MrCrambo assigned

sarathi16 commented 5 years ago

Auditing time :1day

RideSolo commented 5 years ago

Auditing time: 1day

danbogd commented 5 years ago

Auditing time: ~ 2 days.

yuriy77k commented 5 years ago

@sarathi16 assigned

yuriy77k commented 5 years ago

@RideSolo assigned

yuriy77k commented 5 years ago

@danbogd Not assigned. There are enough auditors.

yuriy77k commented 5 years ago

Security Audit Report

1. Summary

ZRX Token smart contract security audit report performed by Callisto Security Audit Department

Token desription:

Symbol      : ZRX
Name        : 0x Protocol Token
Total supply: 1,000,000,000
Decimals    : 18 
Standard    : ERC20

2. In scope

3. Findings

In total, 4 issues were reported including:

No critical security issues were found.

3.1. 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) );

3.2. ERC-20 Compliance

Severity: medium

Description

Code snippet

https://gist.github.com/yuriy77k/9e3f16ce8289f6fafa64c9fee13dfd1f#file-zrxtoken-sol-L60

https://gist.github.com/yuriy77k/9e3f16ce8289f6fafa64c9fee13dfd1f#file-zrxtoken-sol-L70

3.3. Unlimited Allowance

Severity: low

Description

Even if the likelihood of such issue to represent a risk for users is very low, the reimplemented transferFrom with unlimited allowance to an address is not complaint with ERC-20 standard. Users should be aware of it.

Code snippet

https://gist.github.com/yuriy77k/9e3f16ce8289f6fafa64c9fee13dfd1f#file-zrxtoken-sol-L108

3.4. No zero address checking

Severity: low

Description

In functions transfer and transferFrom there are no zero address checking.

Code snippet

https://gist.github.com/yuriy77k/9e3f16ce8289f6fafa64c9fee13dfd1f#file-zrxtoken-sol-L60

https://gist.github.com/yuriy77k/9e3f16ce8289f6fafa64c9fee13dfd1f#file-zrxtoken-sol-L70

4. Conclusion

The audited code has some ERC20 compliance issues.

5. Revealing audit reports

https://gist.github.com/yuriy77k/3a07536517aef3a02b64024db39f5407

https://gist.github.com/yuriy77k/471b6aeffdc8bc2c5c6a406dbe656ec7

https://gist.github.com/yuriy77k/dfa68fb6651cd142bc1ae85c681dc58c