`V3Vault` lacks slippage protection on deposit/withdraw operations

[c4-bot-4]

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L360-L393 https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L881-L891 https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L924-L932

Vulnerability details

Issue Description

In order to deposit assets in V3Vault or to withdraw previously lent assets one of the following functions must be called: deposit, mint, withdraw and redeem.

In all this functions the exchange lending rate lastLendExchangeRateX96 in used to calculated the assets/shares amount that must be given to the users, this is visible in both _deposit and _withdraw functions (which are internally invoked by all the aforementioned functions):

function _deposit(
    address receiver,
    uint256 amount,
    bool isShare,
    bytes memory permitData
) internal returns (uint256 assets, uint256 shares) {
    ...

    (, uint256 newLendExchangeRateX96) = _updateGlobalInterest();

    _resetDailyLendIncreaseLimit(newLendExchangeRateX96, false);

    if (isShare) {
        shares = amount;
        assets = _convertToAssets(
            shares,
            newLendExchangeRateX96,
            Math.Rounding.Up
        );
    } else {
        assets = amount;
        shares = _convertToShares(
            assets,
            newLendExchangeRateX96,
            Math.Rounding.Down
        );
    }

    ...
}

function _withdraw(
    address receiver,
    address owner,
    uint256 amount,
    bool isShare
) internal returns (uint256 assets, uint256 shares) {
    ...

    (
        uint256 newDebtExchangeRateX96,
        uint256 newLendExchangeRateX96
    ) = _updateGlobalInterest();

    if (isShare) {
        shares = amount;
        assets = _convertToAssets(
            amount,
            newLendExchangeRateX96,
            Math.Rounding.Down
        );
    } else {
        assets = amount;
        shares = _convertToShares(
            amount,
            newLendExchangeRateX96,
            Math.Rounding.Up
        );
    }

    ...
}

This lending rate lastLendExchangeRateX96 is variable, it will increase on a second basis while the interest in accrued and may decrease if there is an underwater liquidation (bad debt socialized on all lenders which's handled by _handleReserveLiquidation), because the lending rate is variable it means that when a user submit a tx either by calling deposit or mint to lend assets or withdraw, redeem to withdraw lent asset, he might experience a slippage when the tx is pending in the mempool (as the protocol will be deployed on any EVM) and might receive less assets or shares from the vault.

To illustrate this issue, consider the following scenarios:

- Scenario 1:

1- In the vault, we have the lending rate lastLendExchangeRateX96 = 1.2 * Q96

2- Bob wants to deposit 1000 asset tokens and he's expecting to get approximately 830 shares back from the vault.

3- Bob calls the deposit function.

4- Bob deposit tx remains in the mempool pending execution for some time, and in that time the lending rate did increase lastLendExchangeRateX96 = 1.28 * Q96.

5- When Bob tx goes through he gets minted (1000 * Q96)/(1.28 * Q96) = 781.25 shares which muck less than the ~830 shares that Bob was expecting

So Bob did experience a slippage which make him lose ~50 shares, and the same issue would happen if Bob did call the mint function instead.

- Scenario 2:

1- In the vault, we have the lending rate lastLendExchangeRateX96 = 1.2 * Q96

2- Bob wants to withdraw 1000 asset tokens and he's expecting to burn approximately 830 shares from his balance.

3- Bob calls the withdraw function.

4- While Bob deposit tx remains in the mempool pending execution, a liquidation happens and it's an underwater liquidation and as reserve couldn't cover it fully, a bad debt was generated and lending rate did decrease lastLendExchangeRateX96 = 1.1 * Q96.

5- When Bob tx goes through he will get (1000 * Q96)/(1.1 * Q96) = 909 shares burned from his balance in order to receive his 1000 asset tokens which more that Bob was expecting to burn

Bob did again experience a slippage which make him spend ~70 shares more that what he was intending to withdraw 1000 assets.

In both scenarios, Bob did lose funds due to a change in the lending rate lastLendExchangeRateX96 while his tx was pending in the mempool. And because it's stated in the contest ReadMe that the protocol is expected to be deployed on any EVM compatible chain like Ethereum, Polygon which use tx mempool, this kind of issue might happen frequently to many users.

Impact

The deposit, mint, withdraw and redeem functions lack slippage protection, which can lead to users losing funds when trying to lend or withdraw asset from the V3Vault, and this due to the changes in the the lending rate lastLendExchangeRateX96 after users submit their transactions to the mempool.

Tools Used

Manual review, VS Code

Recommended Mitigation

The deposit, mint, withdraw and redeem functions should include slippage protection parameters provided by the users (either minimum amount received for deposit & redeem function or a maximum amount (shares) used in mint, withdraw).

Assessed type

Context

[c4-pre-sort]

0xEVom marked the issue as sufficient quality report

[c4-pre-sort]

0xEVom marked the issue as duplicate of #281

[c4-judge]

jhsagd76 changed the severity to QA (Quality Assurance)

[c4-judge]

jhsagd76 marked the issue as grade-b

[c4-judge]

This previously downgraded issue has been upgraded by jhsagd76

[c4-judge]

jhsagd76 changed the severity to QA (Quality Assurance)