deposit() incorrect use balanceOf(msg.sender)

[c4-bot-10]

Lines of code

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

Vulnerability details

Vulnerability details

in Vault.deposit() We will limit the user's maximum deposit cannot exceed settings().getMaxDepositInETH().

    function deposit(
        address receiver
    )
        external
        payable
        override
        nonReentrant
        whenNotPaused
        onlyWhiteListed
        returns (uint256 shares)
    {
        if (msg.value == 0) revert InvalidDepositAmount();
        uint256 maxPriceAge = settings().getPriceMaxAge();
        Rebase memory total = Rebase(_totalAssets(maxPriceAge), totalSupply());
        if (
            // Or the Rebase is unititialized
            !((total.elastic == 0 && total.base == 0) ||
                // Or Both are positive
                (total.base > 0 && total.elastic > 0))
        ) revert InvalidAssetsState();
        // Verify if the Deposit Value exceeds the maximum per wallet
        uint256 maxDeposit = settings().getMaxDepositInETH();
        if (maxDeposit > 0) {
            uint256 afterDeposit = msg.value +
@>              ((balanceOf(msg.sender) * _tokenPerETH(maxPriceAge)) / 1e18);
            if (afterDeposit > maxDeposit) revert MaxDepositReached();
        }

....

        uint256 amount = abi.decode(result, (uint256));
        shares = total.toBase(amount, false);
@>      _mint(receiver, shares);
        emit Deposit(msg.sender, receiver, msg.value, shares);
    }

First, we need to calculate afterDeposit = msg.value + ((balanceOf(receiver) * _tokenPerETH(maxPriceAge)) / 1e18). The usage of balanceOf(msg.sender) inside is incorrect because we ultimately deposit is into receiver: _mint(receiver, shares). So, it should be corrected to balanceOf(receiver).

Impact

An incorrect calculation afterDeposit can result in exceeding getMaxDepositInETH or prematurely triggering a MaxDepositReached revert. users may not be able to deposit properly.

Recommended Mitigation

    function deposit(
        address receiver
    )
...
        uint256 maxDeposit = settings().getMaxDepositInETH();
        if (maxDeposit > 0) {
            uint256 afterDeposit = msg.value +
-               ((balanceOf(msg.sender) * _tokenPerETH(maxPriceAge)) / 1e18);
+               ((balanceOf(receiver) * _tokenPerETH(maxPriceAge)) / 1e18);
            if (afterDeposit > maxDeposit) revert MaxDepositReached();
        }
...
        uint256 amount = abi.decode(result, (uint256));
        shares = total.toBase(amount, false);
        _mint(receiver, shares);
        emit Deposit(msg.sender, receiver, msg.value, shares);
    }

Assessed type

Context

[stalinMacias]

In this report looks like there is a missunderstanding of who is a depositor in the Vaults.

The current check is correct because it validates if the caller (the depositor) has surpassed the maxDeposit limit. The check is enforced correctly on the caller because if we look at the onlyWithelisted() modifier, this modifier checks if the caller is whitelisted to deposit on the Vault, as such, that indicates that the depositor is the caller itself.

    function deposit(
        address receiver
    )
        ...
        //@audit => The caller is the depositor that is validated to be whitelisted.
        onlyWhiteListed
        returns (uint256 shares)
    { ... }

    modifier onlyWhiteListed() {
        if (!settings().isAccountEnabled(msg.sender)) revert NoPermissions();
        _;
    }

A whitelisted depositor could decide to distribute his shares among different accounts, but, in the end, the Vault enforces that the whiteslited depositor has not exceeded the maxDeposit, even though the shares are distributed among different accounts.

[c4-judge]

0xleastwood marked the issue as duplicate of #29

[c4-judge]

0xleastwood marked the issue as satisfactory