code-423n4 / 2024-03-revert-lend-findings

6 stars 6 forks source link

Wrong globalLendLimit check #494

Closed c4-bot-8 closed 4 months ago

c4-bot-8 commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L906-L908

Vulnerability details

In V3vault deposit, lend share instead of amount is mistakenly used to check whether the globalLendLimit is exceeded. As time increases, the total max available deposit amount will become more and more.

Impact

As time increases, the total max available deposit amount will become more and more.

Proof of Concept

In white paper:

globalLendLimit: Limits the total lending token amount that can be deposited. It limits new deposits but does not affect existing ones.

and it is used correctly in maxDeposit and maxMint:

function maxDeposit(address) external view override returns (uint256) {
        (, uint256 lendExchangeRateX96) = _calculateGlobalInterest();
        uint256 value = _convertToAssets(totalSupply(), lendExchangeRateX96, Math.Rounding.Up);
        if (value >= globalLendLimit) {
            return 0;
        } else {
            return globalLendLimit - value;
        }
    }
    /// @inheritdoc IERC4626
    function maxMint(address) external view override returns (uint256) {
        (, uint256 lendExchangeRateX96) = _calculateGlobalInterest();
        uint256 value = _convertToAssets(totalSupply(), lendExchangeRateX96, Math.Rounding.Up);
        if (value >= globalLendLimit) {
            return 0;
        } else {
            return _convertToShares(globalLendLimit - value, lendExchangeRateX96, Math.Rounding.Down);
        }
    }

but it used incorrectly in function _deposit:

https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Oracle.sol#L106-L117

    function _deposit(address receiver, uint256 amount, bool isShare, bytes memory permitData)
        internal
        returns (uint256 assets, uint256 shares)
    {
     ···
        if (totalSupply() > globalLendLimit) {
            revert GlobalLendLimit();
        }

        if (assets > dailyLendIncreaseLimitLeft) {
            revert DailyLendIncreaseLimit();
        } else {
            dailyLendIncreaseLimitLeft -= assets;
        }

        emit Deposit(msg.sender, receiver, assets, shares);
    }

because vault is positive rebase token,one share represents more and more amount as time increases.So in fixed globalLendLimit,the total max available deposit amount will become more and more with time increases。

This error breaks the protocol's lend limit, so I consider it is a medium severity finding.

Tool Used

vscode、foundary

Recommended mitigation steps

Use this in check:

function _deposit(address receiver, uint256 amount, bool isShare, bytes memory permitData)
    internal
    returns (uint256 assets, uint256 shares)
{
 ···
      uint256 totalAsset = _convertToAssets(totalSupply(), newLendExchangeRateX96, Math.Rounding.Up);
      if (totalAsset > globalLendLimit) {
          revert GlobalLendLimit();
      }

    if (assets > dailyLendIncreaseLimitLeft) {
        revert DailyLendIncreaseLimit();
    } else {
        dailyLendIncreaseLimitLeft -= assets;
    }

    emit Deposit(msg.sender, receiver, assets, shares);
}

Assessed type

Error

c4-pre-sort commented 4 months ago

0xEVom marked the issue as sufficient quality report

c4-pre-sort commented 4 months ago

0xEVom marked the issue as duplicate of #324

c4-judge commented 3 months ago

jhsagd76 marked the issue as satisfactory