First depositor for the Vault can be front-run and have part of their deposit stolen

[code423n4]

Lines of code

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

Vulnerability details

Description

The first deposit with a totalSupply of zero shares will mint shares equal to the deposited amount.

File: src/vault/Vault.sol

298:     supply == 0
299:            ? assets
300:            : assets.mulDiv(supply, totalAssets(), Math.Rounding.Down);

Link to Code

File: src/vault/Vault.sol

    function deposit(uint256 assets, address receiver)
        public
        nonReentrant
        whenNotPaused
        syncFeeCheckpoint
        returns (uint256 shares)
    {
        if (receiver == address(0)) revert InvalidReceiver();

        uint256 feeShares = convertToShares(
            assets.mulDiv(uint256(fees.deposit), 1e18, Math.Rounding.Down)
        );

        shares = convertToShares(assets) - feeShares;

        if (feeShares > 0) _mint(feeRecipient, feeShares);

        _mint(receiver, shares);

        asset.safeTransferFrom(msg.sender, address(this), assets);

        adapter.deposit(assets, address(this));

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

Link to Code

For calculation Simplicity let's take fees to be zero as of now.

Problems with the code:

1. Integer division negatively affect the user.
2. Can be manipulated to cause a large loss, specifically for first victim.

Impact

It can lead to some part of Fund getting stolen from First Depositor.

Proof of Concept

Consider the following situation:

1. Attacker deposits 1 wei of WETH.
2. Next, Attacker transfers 100 WETH to the contract.
3. Victim deposits 200 WETH.
4. Attacker withdraws 1 share.

Have a look at this table to understand the complete PoC:

	Before	Before		After	After
Tx	totalSupply	balanceOf	sharesToMint	totalSupply	balanceOf
BeforeAttacker deposits 1 wei of WETH.	0	0	1	1	1
Attacker transfers 100 WETH to the contract.	1	1	N/A	1	1 + 100 x 10^18
Victim deposits 200 WETH.	1	1 + 100 x 10^18	=1.99 = 1	2	1 + 300 x 10^18
Attacker withdraws 1 share.	2	1 + 300 x 10^18	N/A	1	1 + 150 x 10^18

Tools Used

Manual Review

Recommended Mitigation Steps

1. Need to Enforce a minimum deposit that can't be withdrawn.
2. So, mint some of the initial amount to the zero address.
3. Most legit first depositors will mint thousands of shares, so not a big cost.

[c4-judge]

dmvt marked the issue as duplicate of #15

[c4-sponsor]

RedVeil marked the issue as sponsor confirmed

[c4-judge]

dmvt marked the issue as satisfactory