Skip to content

Instantly share code, notes, and snippets.

@yuriy77k
Forked from sarathi16/ETH_ZRXToken_report.md
Created April 14, 2019 20:25
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/dfa68fb6651cd142bc1ae85c681dc58c to your computer and use it in GitHub Desktop.
Save yuriy77k/dfa68fb6651cd142bc1ae85c681dc58c to your computer and use it in GitHub Desktop.

Ethereum ZRXToken audit report

Summary

This is the report from a security audit performed on Ethereum ZRXToken by Parthasarathi R.

The audit focused primarily on the security and stability of the ZRXToken’s solidity smart contract.

In scope :

ZRXToken.sol

Findings

In total, 4 issues are found

  • 2 low severity issue

  • 2 minor observations

Security Issues :

1. Prone to Double withdrawal Race condition

Severity: Low

Description:

The ERC20 standard's approve() and transferFrom() functions are vulnerable to a race condition.

Eg :Suppose Alice has approved Bob to spend 100 tokens on her behalf. She then decides to only approve him for 50 tokens and sends a second approve transaction. However, Bob sees that he's about to be downgraded and quickly submits a transferFrom for the original 100 tokens he was approved for. If this transaction gets mined before Alice's second approve, Bob will be able to spend 150 of Alice's tokens.

More detail about the attack is given here.

Recommendation:

A potential fix includes preventing a call to approve if all the previous tokens are not spent through adding a check that the allowed balance is 0.

Either by using require(allowed[msg.sender][_spender] == 0). Or by using the modified version of the ERC20 approve function.

2.Missing function visibility specifier

Severity: Minor Observation

Description :

Almost all the functions in StandardToken contract have no visibility specifier. It is highly recommended to explicitly mention the visibility specifier even though the function is public.

Recommendation :

Mention the appropriate visibility specifier to all the functions in StandardToken contract.

3. SafeMath library implementation

Severity: Minor Observation

Description :

Usage of openZeppelin's safeMath library reduces the complexity of the code for easy readability.

Recommendation :

Use safeMath library for arithmetic validation.

4. No zero address checking

Severity:Low

Description :

Functions transfer() and transferFrom() in StandardToken contract and transferFrom() in UnlimitedAllowanceToken contract failed to check zero address condition.

Recommendation :

Use proper zero address condition check. Eg : require(_to != address(0))

Conclusion
Only two low severity issues are found.

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