GoGoPool contest - dic0de's results

Lines of code

https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/tokens/TokenggAVAX.sol#L165-L178

Vulnerability details

Impact

The TokenggAVAX contract is based on the ERC4626 where the shares are calculated based on the deposit value. By depositing large amount as initial deposit, initial depositor can influence the future depositors value. Shares are minted based on the deposit value as seen here: https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/tokens/TokenggAVAX.sol#L165-L178.

By depositing large amount as initial deposit, first depositor can take advantage over other depositors. Future depositors are forced for huge value of asset to deposit. It is not practically possible for all the users and this could directly affect on the attrition of users towards this system.

The issue is referenced here :

1. https://github.com/sherlock-audit/2022-11-sense-judging/issues/50
2. https://github.com/sherlock-audit/2022-08-sentiment-judging#issue-h-1-a-malicious-early-userattacker-can-manipulate-the-ltokens-pricepershare-to-take-an-unfair-share-of-future-users-deposits

Proof of Concept

https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/tokens/TokenggAVAX.sol#L165-L178

Tools Used

Recommended Mitigation Steps

Consider requiring a minimal amount of share tokens to be minted for the first minter, and send a port of the initial mints as a reserve so that the pricePerShare can be more resistant to manipulation.

Lines of code

https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/MultisigManager.sol#L68-L77

Vulnerability details

Impact

The disableMultisig () function is a privileged function which disables registered multisig. This is done by setting the multisig to false as seen here: https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/MultisigManager.sol#L74.

The function comments states /// @dev this will prevent the multisig from completing validations. The minipool will need to be manually reassigned to a new multisig. As a result, it is not intended that once a multisig is disabled it would be enabled again. With the deployed mechanism of disabling a multisig as seen in disableMultisig () function which would simply set the multisig to false, provides the opportunity for a disabled multisig to be set to true in the future.

If the intention of the project is to ensure that disabled multisigs cannot be re-enabled in future, then it should consider not only setting the multisig to false but also completely unregistering the multisig.

Proof of Concept

https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/MultisigManager.sol#L68-L77

Tools Used

Recommended Mitigation Steps

f the intention of the project is to ensure that disabled multisigs cannot be re-enabled in future, then it should consider not only setting the multisig to false but also completely unregistering the multisig.