GoGoPool contest - 0xLad's results

Lines of code

https://github.com/code-423n4/2022-12-gogopool/blob/main/contracts/contract/tokens/upgradeable/ERC4626Upgradeable.sol#L42-L54 https://github.com/code-423n4/2022-12-gogopool/blob/main/contracts/contract/tokens/upgradeable/ERC4626Upgradeable.sol#L56-L67

Vulnerability details

Impact

The user who deposits first can perform a "donation" attack on the user who deposits afterwards. The second user expects to get D shares for a deposit of D assets, but will get significantly less than D shares.

The issue is present in the deposit() and mint() functions of the ERC4626Upgradeable contract that TokenggAVAX inherits from.

Proof of Concept

1. The first user (Alice) deposits 1 wei of assets and receives 1 wei of shares.
2. The second user (Bob) sends a tx to deposit 100e18 assets into the vault, and expects to receive 100e18 of shares because the asset:share ratio is 1:1.
3. Alice frontruns Bob and donates 51e18 - 1 assets to the vault. After Bob's transaction completes, he receives (100e18) * (1 / 51e18 ) = 1 wei of shares.

Bob deposited 66.2% (100 / 151) of the assets in the vault, but is only entitled to 50% (both he and Alice have 1 wei of shares). Bob can only withdraw 75.5e18 assets and has lost 25% of his deposit.

Recommended Mitigation Steps

Add a minimum size for the first deposit, or add "virtual" assets and shares when doing asset-share conversions. See YieldBox for an example of the virtual asset approach.