GoGoPool contest - tonisives's results

Lines of code

https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/tokens/TokenggAVAX.sol/#L166 https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/tokens/upgradeable/ERC4626Upgradeable.sol/#L121

Vulnerability details

Impact

The attacker can profit from future users' deposits. While the late users will lose part of their funds to the attacker.

Proof of Concept

A well known attack vector for almost all shares based liquidity pool contracts, where an early user can manipulate the price per share and profit from late users' deposits because of the precision loss caused by the rather large value of price per share.

A malicious early user can depositAVAX() with 1 wei of asset token as the first depositor of the ggAVAX, and get 1 wei of shares.

Then the attacker can send 10000e18 - 1 of asset tokens and inflate the price per share from 1.0000 to an extreme value of 1.0000e22 ( from (1 + 10000e18 - 1) / 1) .

As a result, the future user who deposits 19999e18 will only receive 1 wei (from 19999e18 * 1 / 10000e18) of shares token.

They will immediately lose 9999e18 or half of their deposits if they redeem() right after the deposit().

Code Snippet

function depositAVAX() public payable returns (uint256 shares) {
	uint256 assets = msg.value;
	// Check for rounding error since we round down in previewDeposit.
	if ((shares = previewDeposit(assets)) == 0) {
		revert ZeroShares();
	}

link

previewDeposit finally calls

function convertToShares(uint256 assets) public view virtual returns (uint256) {
	uint256 supply = totalSupply; // Saves an extra SLOAD if totalSupply is non-zero.

	return supply == 0 ? assets : assets.mulDivDown(supply, totalAssets());
}

link

Tools Used

Manual Review

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 to the DAO so that the previewDeposit can be more resistant to manipulation.