code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/main/src/erc-4626/ERC4626.sol#L32-L44 https://github.com/code-423n4/2023-05-maia/blob/main/src/erc-4626/ERC4626.sol#L120-L122 https://github.com/code-423n4/2023-05-maia/blob/main/src/erc-4626/ERC4626.sol#L106-L110

Vulnerability details

Impact

The ERC4626.deposit() function is vulnerable to first depositor attack. It can be described as follows:

A malicious early depositor can deposit() with 1 wei of asset token as the first depositor of the Vault, and get 1 wei of shares.

Then the first depositor can send 10000e18 - 1 of asset tokens directly to the contract and inflate the price per share from 1.0000 to an extreme value of 1.0000e22 ( from (1 + 10000e18 - 1) / 1) .

As a result, the future user who deposits 19999e18 will only receive (from 19999e18 * 1 / 10000e18) 1 share token.

They will immediately lose 9999e18 or half of their deposits if they redeem() right after the deposit().

Proof of Concept

    function deposit(uint256 assets, address receiver) public virtual returns (uint256 shares) {
        // Check for rounding error since we round down in previewDeposit.
        require((shares = previewDeposit(assets)) != 0, "ZERO_SHARES");

        // Need to transfer before minting or ERC777s could reenter.
        address(asset).safeTransferFrom(msg.sender, address(this), assets);

        _mint(receiver, shares);

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

        afterDeposit(assets, shares);
    }

https://github.com/code-423n4/2023-05-maia/blob/main/src/erc-4626/ERC4626.sol#L32-L44

    function previewDeposit(uint256 assets) public view virtual returns (uint256) {
        return convertToShares(assets);
    }

https://github.com/code-423n4/2023-05-maia/blob/main/src/erc-4626/ERC4626.sol#L120-L122

    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.mulDiv(supply, totalAssets());
    }

https://github.com/code-423n4/2023-05-maia/blob/main/src/erc-4626/ERC4626.sol#L106-L110

Tools Used

VSCode and Manual Review

Recommended Mitigation Steps

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