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

Audtichain #519

Closed Vestcomp closed 3 years ago

Vestcomp commented 3 years ago

Audit request

... Briefly describe your smart-contract and its main purposes here ...

WHitelist contract, Sale contract and two vesting contracts https://auditchain.finance/private-sale

Source code

https://github.com/Auditchain/Private-Sale

... Give a link to the source code of contracts ...

Disclosure policy

... Do you want us to publish the report as it is or to notify you privately in case of finding critical mistakes? ...NO

... provide your conditions for publishing the report or leave only standard disclosure policy link ...

Standard disclosure policy.

Contact information (optional)

... Provide information to contact you or the smart contract-developer in case high severity issues will be found ...jm@auditchain.com

... Provide information about the media resources of the project you want us to audit (website/ twitter account/ reddit/ telegram channel/ etc.) ...

Platform

... In which network will your contract be deployed? (EOS/TRX/ETC/ETH/CLO/UBQ/something else ) ...Ethereum

yuriy77k commented 3 years ago

@Vestcomp The audit fee is 750 USDT. You may send USDT (ERC20 or BEP20) to: 0xb9662e592f2f0412be62f0833ca463a9b1aabebb or USDT (TRC20) to: TBzUKbek9AYVBwf91ykh3KY4Ushk95SCiB

The estimated auditing time - 7 days after payment.

Vestcomp commented 3 years ago

Hi @yuriy77k we can only send DAI or ETH. Dan you accommodate?

yuriy77k commented 3 years ago

Hi @Vestcomp, you may pay with DAI to: 0xb9662e592f2f0412be62f0833ca463a9b1aabebb

Vestcomp commented 3 years ago

Confirming the following wallet address to send DAI...Please respond...

0xb9662e592f2f0412be62f0833ca463a9b1aabebb

yuriy77k commented 3 years ago

Confirming the following wallet address to send DAI...Please respond...

0xb9662e592f2f0412be62f0833ca463a9b1aabebb

Yes, I confirm

Vestcomp commented 3 years ago

https://etherscan.io/tx/0x3fbd6fbe1ab9c2547df1a6dc5e29905e765fe3868c91d1204b3c8c0cc3b034c7

yuriy77k commented 3 years ago

Thank you. Received

Vestcomp commented 3 years ago

Can you please add bogdanfiedur@gmail.com to all communications? He developed the contracts. Thank you

Vestcomp commented 3 years ago

@yuriy77k can you say if you have seen any serious vulnerabilities so far?

yuriy77k commented 3 years ago

Auditchain Security Audit Report

1. Summary

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

2. In scope

Commit 54b18eb12fe79941bb90bd057ea847fe0264ad8f

3. Findings

In total, 7 issues were reported, including:

3.1. Known vulnerability of ERC-20 token

Severity: low

Description

The DAI token contract has lack of transaction handling mechanism issue. WARNING! This is a very common issue, and it already caused millions of dollars in losses for lots of token users! More details here.

Recommendation

Add the following code to the transfer(_to address, ...) function:

require( _to != address(this) );

3.2. Wrong condition

Severity: medium

Description

In the modifier isNotLocked member of contract Locked the condition hes opposite action as described in comment. It allows any user to bypass checking _from and _to addresses is locked, but only if address _from has admin role the addresses will be checked.

Recommendation

Add ! (NOT) into condition: if (!hasRole(DEFAULT_ADMIN_ROLE, _from))

3.3. Possible wrong arguments

Severity: note

Description

The function burnFrom() member of AuditToken contract checks isNotLocked(msg.sender, msg.sender) but by contract logic it seems to be isNotLocked(user, user). Please, pay attention to it.

3.4. Owner privileges

Severity: owner privileges

Description

The contract owner has DEFAULT_ADMIN_ROLE and may assign any role to any wallets.

The role CONTROLLER_ROLE has following rights in AuditToken, Locked, WhiteList contracts:

  1. Can lock and unlock any address;
  2. Can mint any amount of Auditchain tokens to any address;
  3. Can pause and unpause Auditchain tokens operations;
  4. Can add and remove users from/to whitelist.

Admin (Operator) of Vesting contract has next rights:

  1. revoke() / reinstate() access to continue vesting of tokens of any user.
  2. Add funds to the early investor or team member.

3.5. Unused variable

Severity: note

Description

The _operator declared but is unused.

Recommendation

Remove unused variable.

3.6. Double-check permission

Severity: note

Description

The functions revoke() and reinstate() have isOperator modifier, but also it require the msg.sender == admin, that is the same as required in modifier.

Recommendation

Remove duplicated require(msg.sender == admin) from functions body.

3.7. Inaccurate condition

Severity: note

Description

In the function release() you require releasedAmount <= tokensToSend, but it should be releasedAmount < tokensToSend, because if releasedAmount == tokensToSend than all tokens claimed.

3.8. The keyword emit missed

Severity: note

Description

In the function claimStake() when emit event StakingRewardsReleased the keyword emit was missed.

3.9. An operator may add more tokens than vesting required.

Severity: Low

Description

An operator may accidentally call a function fundVesting() more than once and transfer more tokens than required for vesting. These tokens will be locked on contract forever.

Recommendation

Add require(!fundingCompleted) on begging of function.

4. Security practices

5. Conclusion

The audited smart contract must not be deployed. Reported issues must be fixed prior to the usage of this contract.

It is recommended to adhere to the security practices described in pt. 4 of this report in order to ensure the operability of the contract and prevent any issues which are not directly related to the code of this smart-contract.