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
132 stars 34 forks source link

Auditchain Token #158

Closed yuriy77k closed 5 years ago

yuriy77k commented 5 years ago

Audit request

Burnable Mintable Pausable No Cap Issuing yearly 12,500,000 new tokens to governance contract With ability to be migrated to new contract in the future With ability to lock and unlock ERC20 functions of Token based on user address With ability to refuse to accept tokens sent to this contract by mistake

Token Name

"Auditchain"

Token Ticker

"AUDT"

Number of decimals

18

Initial Supply

250,000,000 + 12,500,000 Example of verified contract can be seen on ropsten testnet

Source code

https://github.com/Vestcomp/TGE

Disclosure policy

jm@auditchain.com

bogdanfiedur@gmail.com

Platform

ETH

Complexity

Low

MrCrambo commented 5 years ago

Auditing time 1 day

yuriy77k commented 5 years ago

@MrCrambo assigned

danbogd commented 5 years ago

Auditing time: 2 day.

gorbunovperm commented 5 years ago

Estimated auditing time is 1 day.

yuriy77k commented 5 years ago

@danbogd @gorbunovperm assigned

danbogd commented 5 years ago

My report is finished.

gorbunovperm commented 5 years ago

My report is finished.

yuriy77k commented 5 years ago

1. Summary

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

https://auditchain.com/

2. In scope

Github commit hash 6ebb925a6f8bf9a744b2c6eedaf71bacf24ddf46

  1. Locked.sol
  2. MigrationAgent.sol
  3. Migrations.sol
  4. Token.sol

3. Findings

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

require( _to != address(this) );

3.2. Missing event call.

Severity: low

Description

In the constructor is no events on the transfer of funds.

Code snippet

https://github.com/Vestcomp/TGE/blob/6ebb925a6f8bf9a744b2c6eedaf71bacf24ddf46/contracts/Token.sol#L42-L45

3.3. Date related issues

Severity: low

Description

It is about

Issuing yearly 12,500,000 new tokens to governance contract

  1. In this contract, the year is determined by dividing the number of seconds since the beginning of the Unix Epoch by the number of seconds in a leap year. In this case 06.02.2019 11:59pm is 2018 year by returnYear() function. And 07.02.2019 12:00am is 2019 year.

  2. If the contract is deployed on February 6 the owner will receive 12'500'000 tokens. And the next day he can call mint() and get another 12'500'000 tokens because the contract will consider that the next year has come.

Code snippet

https://github.com/Vestcomp/TGE/blob/6ebb925a6f8bf9a744b2c6eedaf71bacf24ddf46/contracts/Token.sol#L52

Recommendation

Just save the timestamp of first payment and add to it year value in seconds to figure out if next payment date is come. And then update the date of the last payment.

3.4. There is a possibility of losing funds during the migration process

Severity: low

Description

In the process of migration, all funds of the sender are burned, but only those that he specified as an argument are sent to new contract. The user may not have actual information of the his balance and accidentally burn part of his funds.

Code snippet

https://github.com/Vestcomp/TGE/blob/6ebb925a6f8bf9a744b2c6eedaf71bacf24ddf46/contracts/Token.sol#L86

Recommendation

Do not use value from argument of migrate but set _value = balanceOf(msg.sender);.

3.5. Owners privileges

Severity: low

Description

The owners privileges:

Code snippet

https://github.com/Vestcomp/TGE/blob/6ebb925a6f8bf9a744b2c6eedaf71bacf24ddf46/contracts/Token.sol#L80

https://github.com/Vestcomp/TGE/blob/6ebb925a6f8bf9a744b2c6eedaf71bacf24ddf46/contracts/Token.sol#L103

4. Conclusion

The review of Auditchain Token smart contract did not show any critical issues, but some low severity issues were found.

5. Revealing audit reports

https://gist.github.com/yuriy77k/3148010d397a86f57800b9d98ef83cee

https://gist.github.com/yuriy77k/5a8b0bf7838635ccb090d664fd283eb9

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