Skip to content

Instantly share code, notes, and snippets.

@yuriy77k
Forked from danbogd/ETH_2keynetwork_audit_report.md
Created August 15, 2019 18:07
Show Gist options
  • Save yuriy77k/bf6a51d7397b343aa703670ce929d206 to your computer and use it in GitHub Desktop.
Save yuriy77k/bf6a51d7397b343aa703670ce929d206 to your computer and use it in GitHub Desktop.

2key network audit report.

1. Summary

This document is a security audit report performed by danbogd, where 2key network has been reviewed.

2. In scope

Сommit hash c85f3e1f3a04c56afd28bf673758367cc3df6609.

3. Findings

In total, 8 issues were reported including:

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

No critical security issues were found.

3.1. No checking for zero address

Severity: low

Description

Incoming addresses should be checked for an empty value(0x0 address) to avoid loss of funds or blocking some functionality.

Code snippet

https://github.com/2key/contracts/blob/034b0b6d2713efda29f277c7e81329df9aabb473/contracts/2key/TwoKeyAirdropCampaign.sol#L88 https://github.com/2key/contracts/blob/839a291ef0c240448a079962b1bb153f71cb5919/contracts/2key/token-pools/TokenPool.sol#L24-L25 https://github.com/2key/contracts/blob/839a291ef0c240448a079962b1bb153f71cb5919/contracts/2key/token-pools/TwoKeyCommunityTokenPool.sol#L21-L23 https://github.com/2key/contracts/blob/839a291ef0c240448a079962b1bb153f71cb5919/contracts/2key/token-pools/TwoKeyCommunityTokenPool.sol#L52 https://github.com/2key/contracts/blob/295b13424a21ae9f52844a3a706af02aa4472fd4/contracts/2key/upgradability/UpgradeabilityProxy.sol#L26 https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/donation-campaign-contracts/InvoiceTokenERC20.sol#L36

3.2. 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

https://github.com/2key/contracts/blob/034b0b6d2713efda29f277c7e81329df9aabb473/contracts/2key/ERC20CustomToken.sol#L12 https://github.com/2key/contracts/blob/034b0b6d2713efda29f277c7e81329df9aabb473/contracts/2key/ERC20TokenMock.sol#L12 https://github.com/2key/contracts/blob/295b13424a21ae9f52844a3a706af02aa4472fd4/contracts/2key/acquisition-campaign-contracts/TwoKeyAcquisitionCampaignERC20.sol#L78

3.3. Owner Privileges

Severity: owner previliges

Description

Contract owner allow himself to:

  • upgrade contract and implement any logic in the new contract. And even if the new contract will be audited, at any time possible to change the address of the new contract again to not audited and insecure.

  • to freeze all transfers on ERC for some period of time

Code snippet

https://github.com/2key/contracts/blob/295b13424a21ae9f52844a3a706af02aa4472fd4/contracts/2key/upgradability/UpgradeabilityProxy.sol#L26

https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/singleton-contracts/TwoKeyEconomy.sol#L58

3.5. Transfer prevents transfers of zero value

Severity: medium

Description

The transferFrom function in TwoKeyCampaign is not ERC20 compliant as it stands. From the ERC20 standard: “Note Transfers of 0 values MUST be treated as normal transfers and fire the Transfer event”. This function has a require statement require(_value == 1); that causes execution to revert.

Code snippet

https://github.com/2key/contracts/blob/295b13424a21ae9f52844a3a706af02aa4472fd4/contracts/2key/campaign-mutual-contracts/TwoKeyCampaign.sol#L68

3.5. Transfer prevents spending all tokens

Severity: low

Description

The transferFrom function in TwoKeyCampaign will not let users transfer all of their tokens because of the require statement require(balances[_from] > 0);, which prevents users from decreasing their balance to zero by using the transferFrom function.

Code snippet

https://github.com/2key/contracts/blob/295b13424a21ae9f52844a3a706af02aa4472fd4/contracts/2key/campaign-mutual-contracts/TwoKeyCampaign.sol#L69

3.6. Known vulnerabilities of ERC-20 token

Severity: low

Description

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.

3.7. Multi Transfer arrays length check.

Severity: low

Description

There are several input arrays, but no check for the same length. In this case it is possible to skip some parametres.

Code snippet

https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/singleton-contracts/TwoKeyCongress.sol#L98 https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/singleton-contracts/TwoKeyExchangeRateContract.sol#L61 https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/singleton-contracts/TwoKeyFactory.sol#L78 https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/singleton-contracts/TwoKeyFactory.sol#L187

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