Skip to content

Instantly share code, notes, and snippets.

@yuriy77k
Created June 29, 2019 07:13
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/deebe5089b27642706b8fcd4c7e0bc28 to your computer and use it in GitHub Desktop.
Save yuriy77k/deebe5089b27642706b8fcd4c7e0bc28 to your computer and use it in GitHub Desktop.

Enjin Coin audit report.

1. Summary

This document is a security audit report performed by danbogd, where Enjin Coin has been reviewed.

2. In scope

Сommit hash .

3. Findings

In total, 7 issues were reported including:

  • 1 medium severity issues
  • 3 low severity issues
  • 2 owner privileges (ability of owner to manipulate contract, may be risky for investors)..
  • 1 notes.

No critical security issues were found.

3.1. Allowance Approval

Severity: medium

Description

Following ERC-20 final description:

"NOTE: To prevent attack vectors like the one described here and discussed here, clients SHOULD make sure to create user interfaces in such a way that they set the allowance first to 0 before setting it to another value for the same spender. THOUGH The contract itself shouldn't enforce it, to allow backwards compatibility with contracts deployed before.

The implemented approve function:

Do not preserve backwards compatibility since it enforces the allowance values to be set.

Do not throw in case if the following condition is true require (_value = 0 && allowed[msg.sender][_spender] = 0) and return false, users might not notice that the changes didn't occur, and external contract calls to this function will highlight many other issues.

Code snippet

function approve, line 182.

require(_value == 0 || allowance[msg.sender][_spender] == 0);

3.2. 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 into a function transfer(address _to, ... ) following code:

require( _to != address(this) );

3.3. Owner Privileges

Severity: owner previliges

Description

Contract owner allow himself to:

  • Evacuate Tokens at any time (line 275).

     function withdrawTokens(IERC20Token _token, address _to, uint256 _amount)
     public
     ownerOnly
     validAddress(_token)
     validAddress(_to)
     notThis(_to)
     {
     assert(_token.transfer(_to, _amount));
     } 
    
  • can transfer tokens before the end of the crowdfund (line 478).

     function allowTransfers() ownerOnly {
     isReleasedToPublic = true;
     } 
    

3.4. No checking for zero address

Severity: low

Description

In this constructor there are no checking for zero address.

Code snippet

Lines 348-351

    crowdFundAddress = _crowdFundAddress;
    advisorAddress = _advisorAddress;
    enjinTeamAddress = _enjinTeamAddress;
    incentivisationFundAddress = _incentivisationFundAddress;

3.5. ERC20 Compliance.

Severity: low

Description

Accroding to ERC20 standard, when initializing a token contract if any token value is set to any given address a transfer event should be emited.

Code snippet

Line 352-353

    balanceOf[_crowdFundAddress] = minCrowdsaleAllocation + maxPresaleSupply; 
    balanceOf[_incentivisationFundAddress] = incentivisationAllocation; 

3.6. Incorect endtime.

Severity: note

Description

Before deploy contract update the endtime.

Code snippet

Line 311.

   uint256 constant public endTime = 1509494340;

4. Conclusion

The review did not show any critical issues, some of medium and low severity issues were found.

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