Skip to content

Instantly share code, notes, and snippets.

@MrCrambo
Last active June 19, 2018 22:49
Show Gist options
  • Save MrCrambo/553147e9d3a13b50e9233f097ecae386 to your computer and use it in GitHub Desktop.
Save MrCrambo/553147e9d3a13b50e9233f097ecae386 to your computer and use it in GitHub Desktop.

ETC TokenMint audit report.

Summary

This is the report from a security audit performed on ETC TokenMint by Il Kadyrov.

The audit focused primarily on the security of TokenMint contracts.

In scope

  1. https://github.com/ethereumproject/TokenMint/blob/master/contracts/metaTokenmint.sol
  2. https://github.com/ethereumproject/TokenMint/blob/master/contracts/registry.sol
  3. https://github.com/ethereumproject/TokenMint/blob/master/contracts/deployed/crowdsale.sol
  4. https://github.com/ethereumproject/TokenMint/blob/master/contracts/deployed/icoMachine.sol
  5. https://github.com/ethereumproject/TokenMint/blob/master/contracts/deployed/manReg.sol

Findings

In total, 12 issues were reported including:

  • 4 high severity issues.

  • 3 medium severity issues.

  • 3 low severity issues.

  • 2 minor observation.

Security issues

1. Wrong onlyOwner modifier in file registry.sol(lines 25-29).

Severity: high

Description

onlyOwner modifier has different structure in each smart contract, need to create one smart contract and use it in each other and in registry.sol it will not throw as, if we are not owner.

Recommendation

Rewrite onlyOwner modifier and use same in each smart contract or better to use OpenZeppelins' smart contracts. modifier onlyOwner() { require(msg.sender == owner); _; }

2. Wrong costs modifier in files registry.sol(lines 33-37) and manReg.sol(28-32).

Severity: high

Description

Modifier costs will not throw, if sent value less than price.

Recommendation

Write using require or use throw if msg.value less than price.

3. Wrong onlyAdmin modifier in files registry.sol (lines 175-185), manReg.sol(lines 220-226), metaTokenmint.sol(lines 114-120).

Severity: high

Description

Modifier onlyAdmin will not throw, if msg.sender is admin because of incorrect code.

Recommendation

Write using require or throw after for loop, if there is no admin

4. No checking if initialSupply greater than fundingGoal in icoMachine.sol (lines 54-70).

Severity: high

Description

Modifier onlyAdmin will not throw, if msg.sender is admin because of incorrect code.

Recommendation

Write using require or throw after for loop, if there is no admin

5. Unsafe math.

Severity: medium

Description

In each audited files need to use SafeMath, because there are possibilities to get overflow or underflow.

Recommendation

Use SafeMath from OpenZeppelin.

6. Rewrite withdraw function for ptotecting from spending gas on transactions in registry.sol(lines 120-130) and manReg.sol(105-115).

Severity: medium

Description

There are possibility to spend gas for transaction, if it will revert in if statement.

Recommendation

Rewrite function with using require instead of if statement.

7. Rewrite safeWithdrawal function for ptotecting from spending gas on transactions in crowdsale.sol(lines 61-82).

Severity: medium

Description

There are possibility to spend gas for transaction, if it will revert in one of two if statements.

Recommendation

Rewrite function with using require instead of if statement.

8. Need to use require instead of assert icoMachine.sol(line 60).

Severity: low

Description

Used assert for checking if it's not zero address

Recommendation

Rewrite using require.

9. Old solidity version.

Severity: low

Description

Used solidity version is old.

Recommendation

Need to use one of the latest version of solidity.

10. Not actual suicide function in icoMachine.sol.

Severity: low

Description

There are new selfdestruct function instead of suicide.

Recommendation

Need to change suicide function to selfdestruct.

11. Wrong variable name in crowdsale.sol.

Severity: not a security issue

Description

Used existing values as params name in event. (line 13)

Recommendation

Change variables names.

12. Wrong variable name in metaTokenmint.sol.

Severity: not a security issue

Description

Used variable with name kontract. (line 40)

Recommendation

Change variable name to correct one.

Specification

This smart contract has some high severity vulnerabilities.

Any further changes to the contracts will leave them in unaudited state.

Conclusion

Four high severity vulnerabilities were detected. The reported issues can directly hurt the TokenMint smart contracts.

You need to fix all high severity issues and we highly recommend you to complete other bug bounty before use.

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