Redacted Cartel contest - 0xLad's results

Lines of code

https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/PirexERC4626.sol#L60-L77 https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/PirexERC4626.sol#L80-L97

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 PirexERC4626 contract. The issue is also present in depositGmx() and depositGlp()

Proof of Concept

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

Bob deposited for 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.5 * 1e18 assets and has lost ~25% of his original deposit.

Recommended Mitigation Steps

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

Lines of code

https://github.com/code-423n4/2022-11-redactedcartel/blob/03b71a8d395c02324cb9fdaf92401357da5b19d1/src/vaults/AutoPxGmx.sol#L242-L247 https://github.com/code-423n4/2022-11-redactedcartel/blob/03b71a8d395c02324cb9fdaf92401357da5b19d1/src/PirexRewards.sol#L373

Vulnerability details

Impact

Vault rewards may be drained through a combination of claimRewards(), compound(), and uniswap trades by the attacker.

Proof of Concept

1. Attacker calls claimRewards({producerToken: WETH, user: vault}) to get WETH into the vault.
2. Attacker calls compound({fee: poolFee, amountOutMinimum: 1, sqrtPriceLimitX96: 0, optOutIncentive: true}) . The attacker sandwiches this transaction with two transactions which purchase and sell WETH.
3. The vault loses most of its WETH rewards.

Recommended Mitigation Steps

Only allow compound() to be called by trusted operators. Additionally, consider calculating amountOutMinimum based on chainlink oracle prices.