GoGoPool contest - TomJ's results

Lines of code

https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/tokens/TokenggAVAX.sol#L169 https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/tokens/TokenggAVAX.sol#L225-L227 https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/tokens/upgradeable/ERC4626Upgradeable.sol#L132-L134 https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/tokens/upgradeable/ERC4626Upgradeable.sol#L123

Vulnerability details

Impact

A malicious early user can manipulate the asset per share price which will cause loss of funds for future user and early user will gain profit from future user's deposit.

Proof of Concept

• A malicious early user will deposit 1 nAVAX as first depositor of TokenggAVAX contract and mint 1 share
• A malicious user now transfers 10K (or more) AVAX directly to TokenggAVAX contract to inflate the share's price
• Now any future user have to deposit 10001 tokens or more to deposit and will receive only 1 share in return
• Also future user will immediately lose part of their deposits if they redeem() right after the deposit()

Contract: TokenggAVAX.sol L166-L178
	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();
		}

		emit Deposit(msg.sender, msg.sender, assets, shares);

		IWAVAX(address(asset)).deposit{value: assets}();
		_mint(msg.sender, shares);
		afterDeposit(assets, shares);
	}

Contract: TokenggAVAX.sol L225-L227
	function previewDeposit(uint256 assets) public view override whenTokenNotPaused(assets) returns (uint256) {
		return super.previewDeposit(assets);
	}

Contract: ERC4626Upgradeable.sol L132-L134
	function previewDeposit(uint256 assets) public view virtual returns (uint256) {
		return convertToShares(assets);
	}

Contract: ERC4626Upgradeable.sol L120-L124
	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());
	}

Tools Used

Manual Analysis

Recommended Mitigation Steps

Consider requiring a minimal amount of share tokens to be minted for the first minter or implement logic such as Uniswap V2 does. Reference: https://github.com/Uniswap/v2-core/blob/ee547b17853e71ed4e0101ccfd52e70d5acded58/contracts/UniswapV2Pair.sol#L119-L124