Audit: token-operating smart contracts

[RobertBobor]

Audit request

We would like to have 4 smart contracts to be audited:

vesting/VestingFactory.sol staking/Staking.sol token/Launchpad.sol generic/TimeMultisig.sol

Descriptions for each of them can be found in README files in corresponding folders.

Source code

https://github.com/VIS-DEI/smart-contracts-public/tree/main/contracts

Payment plan

• [ ] Standard (standard projects such as ICOs, ERC20, ERC223, ERC721, ERC721A tokens and NFTs fall in this category - 2 auditors. $600 + $1.2 per line of code / minimum $1200)
• [x] Advanced (non-standard projects requiring more careful review - 3 auditors. $1200 + $1.5 per line / minimum $2400)
• [ ] Corporate (projects that require post-launch support - 3 auditors - Callisto Team will handle the bugbounty and provide a 1 month long period of technical support regarding the necessary security enhancement procedures and track/highlight any issues and potential threats - this plan allows to further apply for Callisto DAPP Insurance program)

Disclosure policy

Please notify me privately.

Contact information (optional)

robert.bobor@dei.cz

Platform

ETH

[yuriy77k]

VIS-DEI Security Audit Report v2

1. Summary

VIS-DEI smart contract security audit report performed by Callisto Security Audit Department

2. In scope

Commit: 93135602752eff72097f602dae44b0d1bed5986d

generic:

• TimeMultisig.sol
• Pausable.sol

staking:

• Staking.sol
• IStaking.sol

token:

• Launchpad.sol
• Vis.sol

vesting:

• VestingFactory.sol

3. Findings

In total, 1 issue were reported, including:

• 0 high severity issues.

• 1 medium severity issue.

• 0 low severity issues.

In total, 5 notes were reported, including:

• 1 minor observation.

• 4 owner privileges.

3.1. Remove owner issues

Severity: medium

Description

There are multiple issues in the function removeOwner:

1. The ownersLength initiated before removing the owner, so condition if (requiredApprovals > ownersLength) will be always false since you compare requiredApprovals with the old ownersLength.
2. To change requirements you call changeRequirement(ownersLength); which will fail, because it requires enough approvals.

Code snippet

• https://github.com/VIS-DEI/smart-contracts-public/blob/93135602752eff72097f602dae44b0d1bed5986d/contracts/generic/TimeMultisig.sol#L159
• https://github.com/VIS-DEI/smart-contracts-public/blob/93135602752eff72097f602dae44b0d1bed5986d/contracts/generic/TimeMultisig.sol#L167
• https://github.com/VIS-DEI/smart-contracts-public/blob/93135602752eff72097f602dae44b0d1bed5986d/contracts/generic/TimeMultisig.sol#L168

Recommendation

Replace this part https://github.com/VIS-DEI/smart-contracts-public/blob/93135602752eff72097f602dae44b0d1bed5986d/contracts/generic/TimeMultisig.sol#L166-L169 with:

        ownersLength--;
        if (requiredApprovals > ownersLength){
            _changeRequirement(ownersLength);
        }

3.2. For dust transactions, the current price would return zero allowing arbitrary wallets to buy tokens without paying.

Severity: minor observation

Description

For dust amount of tokens, the function currentPrice() would always be zero allowing wallets to buy tokens without paying any USDT tokens.

Code Snippet

• https://github.com/VIS-DEI/smart-contracts-public/blob/93135602752eff72097f602dae44b0d1bed5986d/contracts/token/Launchpad.sol#L33-L43

Recommendation

Add in the and of function currentPrice following requirement:

    require(price != 0, "Too small amount");

3.3. Owner privileges

Severity: owner privileges

Description

1. Any owner can Pause/unpause Staking and Launchpad contracts.
2. Set interest in a Staking contract.
3. Withdraw tokens from the Staking contract (including users' deposits).
4. Withdraw tokens from Launchpad contract.

4. Security practices

• [x] Open-source contact.
• [ ] The contract should pass a bug bounty after the completion of the security audit.
• [ ] Public testing.
• [ ] Automated anomaly detection systems. - NOT IMPLEMENTED. A simple anomaly detection algorithm is recommended to be implemented to detect behavior that is atypical compared to normal for this contract. For instance, the contract must halt deposits in case a large amount is withdrawn in a short period until the owner or the community of the contract approves further operations.
• [ ] Multisig owner account.
• [x] Standard ERC20-related issues. - IMPLEMENTED. It is known that every contract can potentially receive an unintended ERC20-token deposit without the ability to reject it even if the contract is not intended to receive or hold tokens. As a result, it is recommended to implement a function that will allow extracting any arbitrary number of tokens from the contract.
• [ ] Crosschain address collisions. ETH, ETC, CLO, etc. It is possible that a transaction can be sent to the address of your contract at another chain (as a result of a user mistake or some software fault). It is recommended that you deploy a "mock contract" that would allow you to withdraw any tokens from that address or prevent any funds deposits. Note that you can reject transactions of native tokens deposited, but you can not reject the deposits of ERC20 tokens. You can use this source code as a mock contract: extractor contract source code. The address of a new contract deployed using CREATE (0xf0) opcode is assigned following this scheme keccak256(rlp([sender, nonce])). Therefore you need to use the same address that was originally used at the main chain to deploy the mock contract at a transaction with the nonce that matches that on the original chain. Example: If you have deployed your main contract with address 0x010101 at your 2021th transaction then you need to increase your nonce of 0x010101 address to 2020 at the chain where your mock contract will be deployed. Then you can deploy your mock contract with your 2021th transaction, and it will receive the same address as your mainnet contract.

5. Conclusion

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

Staking contract do not guarantee users that they will be able to withdraw their deposits if tokens for interest are not added by the owner.

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

6. Revealing audit reports

• https://gist.github.com/oipkasec/0c5334cafe3692fca06b3b1337012846
• https://gist.github.com/chhajershrenik/2e7fa21ffe742526af4ab1510b73761e
• https://gist.github.com/greywolf12/b47c77e6a15b31ee6d5758ddb6b9874a

[yuriy77k]

VIS-DEI Security Audit Report v3

1. Summary

VIS-DEI smart contract security audit report performed by Callisto Security Audit Department

2. In scope

Commit: 4f036260dce9a439c16afcb3dfd298535483e2c7

generic:

• TimeMultisig.sol
• Pausable.sol

staking:

• Staking.sol
• IStaking.sol

token:

• Launchpad.sol
• Vis.sol

vesting:

• VestingFactory.sol

3. Findings

In total, 0 issues were reported, including:

• 0 high severity issues.

• 0 medium severity issues.

• 0 low severity issues.

In total, 4 notes were reported, including:

• 0 minor observations.

• 4 owner privileges.

3.1. Owner privileges

Severity: owner privileges

Description

1. Any owner can Pause/unpause Staking and Launchpad contracts.
2. Set interest in a Staking contract.
3. Withdraw tokens from the Staking contract (including users' deposits).
4. Withdraw tokens from Launchpad contract.

4. Security practices

• [x] Open-source contact.
• [ ] The contract should pass a bug bounty after the completion of the security audit.
• [ ] Public testing.
• [ ] Automated anomaly detection systems. - NOT IMPLEMENTED. A simple anomaly detection algorithm is recommended to be implemented to detect behavior that is atypical compared to normal for this contract. For instance, the contract must halt deposits in case a large amount is withdrawn in a short period until the owner or the community of the contract approves further operations.
• [ ] Multisig owner account.
• [x] Standard ERC20-related issues. - IMPLEMENTED. It is known that every contract can potentially receive an unintended ERC20-token deposit without the ability to reject it even if the contract is not intended to receive or hold tokens. As a result, it is recommended to implement a function that will allow extracting any arbitrary number of tokens from the contract.
• [ ] Crosschain address collisions. ETH, ETC, CLO, etc. It is possible that a transaction can be sent to the address of your contract at another chain (as a result of a user mistake or some software fault). It is recommended that you deploy a "mock contract" that would allow you to withdraw any tokens from that address or prevent any funds deposits. Note that you can reject transactions of native tokens deposited, but you can not reject the deposits of ERC20 tokens. You can use this source code as a mock contract: extractor contract source code. The address of a new contract deployed using CREATE (0xf0) opcode is assigned following this scheme keccak256(rlp([sender, nonce])). Therefore you need to use the same address that was originally used at the main chain to deploy the mock contract at a transaction with the nonce that matches that on the original chain. Example: If you have deployed your main contract with address 0x010101 at your 2021th transaction then you need to increase your nonce of 0x010101 address to 2020 at the chain where your mock contract will be deployed. Then you can deploy your mock contract with your 2021th transaction, and it will receive the same address as your mainnet contract.

5. Conclusion

The audited smart contract can be deployed. No security issues were found during the audit.

Users should pay attention to owners' privileges.

Staking contract do not guarantee users that they will be able to withdraw their deposits if tokens for interest are not added by the owner.

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

6. Previous audit reports

• https://gist.github.com/yuriy77k/b04ca6c04c8d818ee58341fbb0d4e462
• https://gist.github.com/yuriy77k/1e7e39f84135207d090154967357cc57