Popcorn contest - giovannidisiena's results

Lines of code

https://github.com/code-423n4/2023-01-popcorn/blob/main/src/vault/Vault.sol#L134-L158

Vulnerability details

Impact

The first depositor may not receive shares in exchange for their assets if their transaction is front-run and the total asset amount has been manipulated.

Proof of Concept

The amount of shares to mint in exchange for a given number of assets is calculated by:

uint256 supply = totalSupply();
return (supply == 0
    ? assets
    : assets.mulDiv(supply, totalAssets(), Math.Rounding.Down)
);

An attacker can exploit this by front-running the first depositor's transaction, depositing 1 wei of an asset and transferring a large number of tokens on behalf of the vault. This will cause the share price to be greatly inflated which can lead to rounding down of the victim's share calculation.

Example exploit steps:

1. Attacker deposits 1 wei of WETH for 1 share
2. Attacker calls AdapterBase::mint with the vault contract as the receiver, transferring 100 WETH
3. Victim deposits 200 WETH for 1 share (due to rounding)
4. Attacker withdraws 1 share, profiting 50 WETH

Tools Used

Manual Review

Recommended Mitigation Steps

As stated in other similar reports, one solution to this problem is to burn the first 1000 shares thereby increasing the cost to perform this attack by the same factor. Additionally, ensure the number of shares is non-zero to prevent an attacker from stealing all the funds in the case where subsequent deposits are less than Vault::totalAssets:

require(shares != 0, "No shares minted");