code-423n4 / 2024-05-bakerfi-findings

4 stars 4 forks source link

First depositor can abuse exchange rate to steal funds from later depositors #46

Closed c4-bot-2 closed 5 months ago

c4-bot-2 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-05-bakerfi/blob/59b1f70cbf170871f9604e73e7fe70b70981ab43/contracts/core/Vault.sol#L280

Vulnerability details

Vulnerability details

Classic issue with vaults. First depositor can deposit a single wei then donate to the vault to greatly inflate share ratio. Due to truncation when converting to shares this can be used to steal funds from later depositors.

    function deposit(
        address receiver
    )
...
    function deposit(
        address receiver
    )
        external
        payable
        override
        nonReentrant
        whenNotPaused
        onlyWhiteListed
        returns (uint256 shares)
    {
...
        Rebase memory total = Rebase(_totalAssets(maxPriceAge), totalSupply());
...
        uint256 amount = abi.decode(result, (uint256));
@>       shares = total.toBase(amount, false);
        _mint(receiver, shares);

total.toBase formula: amount * totalShares / totalAsset

_totalAssets donation through strategy, like AAVE3.supply(onBehalfOf=valult)

    function _totalAssets(uint256 priceMaxAge) private view returns (uint256 amount) {
        amount = _strategy.deployed(priceMaxAge);
    }

Consider the following scenario. Alice wants to deposit 2M 1e6 USDC to vault. Bob observes Alice's transaction, frontruns to deposit 1 wei USDC to mint 1 wei share, and donation 1 M 1e6 USDC to the vault. Alice's transaction is executed, since _totalAsset = 1M 1e6 + 1 and totalSupply = 1, Alice receives 2M 1e6 1 / (1M 1e6 + 1) = 1 share. The vault now has 3M*1e6 +1 assets and distributed 2 shares. Bob profits 0.5 M and Alice loses 0.5 M USDC.

Impact

First depositor can abuse exchange rate to steal funds from later depositors

Recommended Mitigation

Either during creation of the vault or for first depositor, lock a small amount of the deposit to avoid this.

Assessed type

Context

c4-judge commented 5 months ago

0xleastwood marked the issue as duplicate of #39

c4-judge commented 5 months ago

0xleastwood marked the issue as satisfactory