Popcorn contest - MyFDsYours's results

Lines of code

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

Vulnerability details

Impact

This is a well-known attack vector for contracts that utilize pricePerShare for accounting : users can loss their deposits.

Proof of Concept

https://github.com/code-423n4/2022-04-pooltogether-findings/issues/44

Vault.sol#L147

        shares = convertToShares(assets) - feeShares;

Vault.sol#L293-L300

    /**
     * @notice Amount of shares the vault would exchange for given amount of assets, in an ideal scenario.
     * @param assets Exact amount of assets
     * @return Exact amount of shares
     */
    function convertToShares(uint256 assets) public view returns (uint256) {
        uint256 supply = totalSupply(); // Saves an extra SLOAD if totalSupply is non-zero.

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

A malicious early user can call deposit() with 1 wei as the first depositor of the Vault.sol contract and get 1 wei of shares token.

Then the attacker can send for example (100e18 - 1) directly to the contract without using the deposit function, and inflate the price per share from 1 to an extreme value of 100e18.

if a future user will deposit (100e18-1) token or less he will not receive any shares due to rounding calculation. He will immediately lose his deposit value.

Tools Used

Reading the code

Recommended Mitigation Steps

Consider requiring a minimal amount of share tokens to be minted for the first minter, and send a fixed value of the initial mints as a reserve so that the pricePerShare can be more resistant to manipulation.