V3Vault::_deposit incorrectly validates the global lending limit and allows borrowing of assets above the limit

[c4-bot-5]

Lines of code

https://github.com/revert-finance/lend/blob/audit/src/V3Vault.sol#L961

Vulnerability details

Vulnerability details

V3Vault applies a global limit on the total amount of assets that can be deposited for borrowing. This limit is enforced through the globalLendLimit state variable, set by the contract owner.

This is how the limit is applied in the _deposit() function:

https://github.com/revert-finance/lend/blob/audit/src/V3Vault.sol#L961-L962

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

        // check for global limit
        if (totalSupply() + shares > globalLendLimit) {
            revert GlobalLendLimit();
        }

        // check for daily limit
        if (assets > dailyLendIncreaseLimitLeft) {
            revert DailyLendIncreaseLimit();
        }

        ....
    }

If you look closely you can see that globalLendLimit is compared against the total shares. This is wrong since globalLendLimit is measured in terms of assets, not shares. This can be further validated by looking at the maxDeposit() & maxMint() functions:

https://github.com/revert-finance/lend/blob/audit/src/V3Vault.sol#L322

function maxMint(address) external view override returns (uint256) {
        ....
        uint256 value = _convertToAssets(totalSupply(), lendExchangeRateX96, Math.Rounding.Up);
        if (value >= globalLendLimit) { // <---- value is in assets
            return 0;
        ....
    }

https://github.com/revert-finance/lend/blob/audit/src/V3Vault.sol#L306

 function maxDeposit(address) external view override returns (uint256) {
        ....
        uint256 value = _convertToAssets(totalSupply(), lendExchangeRateX96, Math.Rounding.Up);
        if (value >= globalLendLimit) { // <---- value is in assets
            return 0;
        ....
    }

In time as loan fees accrue the shares to assets ratio will increase, which means that fewer shares would equal greater amount of assets. As a result the global limit will be evaluated against a smaller amount (shares) than it actually should (assets)

Impact

Comparing global debt limit against shares leads to more assets being deposited for borrowing than it should be allowed, breaking important protocol invariant.

Recommended Mitigation

Compare the global lend limit in _deposit against the assets, not the shares:

if (totalAssets() + assets > globalLendLimit) {
     revert GlobalLendLimit();
 }

Assessed type

Invalid Validation

[c4-sponsor]

kalinbas (sponsor) confirmed

[jhsagd76]

After referring to the original issue https://github.com/code-423n4/2024-03-revert-lend-findings/issues/324 , I am more inclined to label it as unmitigated rather than a mitigation error, therefore I am changing this issue to unmitigated.

Because their root causes are similar.

[c4-judge]

jhsagd76 marked the issue as unmitigated

[jhsagd76]

Pls add the MR-M-12 label.

[c4-judge]

jhsagd76 marked the issue as satisfactory

[thebrittfactor]

Label added at request of judge.