Skip to content

Instantly share code, notes, and snippets.

@yuriy77k
Forked from RideSolo/ETH_zrx_report.md
Created April 14, 2019 08:59
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/3a07536517aef3a02b64024db39f5407 to your computer and use it in GitHub Desktop.
Save yuriy77k/3a07536517aef3a02b64024db39f5407 to your computer and use it in GitHub Desktop.

ZRX Token Audit Report.

1. Summary

This document is a security audit report performed by RideSolo, where ZRX Token has been reviewed.

Token desription:

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

2. In scope

  • ZRXToken.sol github gist 9e3f16ce8289f6fafa64c9fee13dfd1f.

3. Findings

3 issues were reported:

  • 1 medium severity issue.
  • 2 low severity issues.

3.1. ERC-20 Compliance

Severity: medium

Description

  • Following EIP-20 specifications transfer should throw when the msg.sender doesn't have enough fund.
  • Same as previously following the specifications transferFrom should throw and not return false if the _from address doesn't have enough of fund or if the allowed value isn't enough to cover the transaction _value.

Code snippet

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

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

3.2. Unlimited Allowance

Severity: low

Description

Even if the likelyhood 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.3. 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) );

Conclusion

The audited code has some ERC20 compliance issues.

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