First depositor for both the `Vault` contract's vault and corresponding adapter's used vault can manipulate share price

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-popcorn/blob/main/src/vault/Vault.sol#L285-L287 https://github.com/code-423n4/2023-01-popcorn/blob/main/src/vault/adapter/abstracts/AdapterBase.sol#L247-L252 https://github.com/code-423n4/2023-01-popcorn/blob/main/src/vault/adapter/beefy/BeefyAdapter.sol#L108-L115 https://github.com/beefyfinance/beefy-contracts/blob/master/contracts/BIFI/vaults/BeefyVaultV6.sol#L69-L71

Vulnerability details

Impact

When the user is the first depositor for both the Vault contract's vault and the corresponding adapter's used vault, this user is able to manipulate the Vault contract's vault's share price by minting 1 share of the Vault contract's vault, which deposits 1 wei of the asset token into the adapter's used vault, and then transferring a large asset amount to the adapter's used vault. This can greatly increase the adapter's used vault's asset balance, such as beefyVault.balance() used in the BeefyAdapter._totalAssets function, and hugely inflate the values returned by the following AdapterBase.totalAssets and Vault.totalAssets functions. The next user, who deposits an asset amount that is less than the large asset balance owned by the adapter's used vault at that moment, will mint 0 share of the Vault contract's vault due to rounding when calling the Vault.convertToShares function and lose the deposited asset amount to the first depositor because the first depositor still owns the only share of the Vault contract's vault.

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

    function totalAssets() public view returns (uint256) {
        return adapter.convertToAssets(adapter.balanceOf(address(this)));
    }

https://github.com/code-423n4/2023-01-popcorn/blob/main/src/vault/adapter/abstracts/AdapterBase.sol#L247-L252

    function totalAssets() public view override returns (uint256) {
        return
            paused()
                ? IERC20(asset()).balanceOf(address(this))
                : _totalAssets();
    }

https://github.com/code-423n4/2023-01-popcorn/blob/main/src/vault/adapter/beefy/BeefyAdapter.sol#L108-L115

    function _totalAssets() internal view override returns (uint256) {
        return
            underlyingBalance.mulDiv(
                beefyVault.balance(),
                beefyVault.totalSupply(),
                Math.Rounding.Down
            );
    }

https://github.com/beefyfinance/beefy-contracts/blob/master/contracts/BIFI/vaults/BeefyVaultV6.sol#L69-L71

    function balance() public view returns (uint) {
        return want().balanceOf(address(this)).add(IStrategy(strategy).balanceOf());
    }

Proof of Concept

The following steps can occur for the described scenario.

1. Alice is the first depositor for both the Vault contract's vault and the corresponding adapter's used vault.
2. She calls the Vault.mint function to mint 1 share and deposit 1 wei of the asset token into the adapter's used vault.
3. Then, she transfers a large asset amount, such as 10e24, to the adapter's used vault.
4. Bob calls the Vault.deposit function to deposit a somewhat large asset amount, such as 10e20, but 0 share of the Vault contract's vault is minted to him.
5. Alice calls the Vault.redeem function to redeem the only share of the Vault contract's vault and receive all of the deposited asset amounts, including Bob's.

Tools Used

VSCode

Recommended Mitigation Steps

When the first deposit occurs, a pre-determined minimum number of shares of the Vault contract's vault can be minted to address(0) before minting shares to the first depositor, which is similar to Uniswap V2's implementation.

[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 partial-50

[c4-judge]

dmvt marked the issue as satisfactory