Revert Lend - Topmark's results

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L906 https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L301-L320

Vulnerability details

Impact

Deposit would Not Revert when Total Supply is too Excessive to handle globalLendLimit and in addition It would Revert when Total Supply is within accurate range, this problem is due to oversight in validation implementation

Proof of Concept

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);
        }

        if (permitData.length > 0) {
            (ISignatureTransfer.PermitTransferFrom memory permit, bytes memory signature) =
                abi.decode(permitData, (ISignatureTransfer.PermitTransferFrom, bytes));
            permit2.permitTransferFrom(
                permit, ISignatureTransfer.SignatureTransferDetails(address(this), assets), msg.sender, signature
            );
        } else {
            // fails if not enough token approved
            SafeERC20.safeTransferFrom(IERC20(asset), msg.sender, address(this), assets);
        }

        _mint(receiver, shares);

>>>        if (totalSupply() > globalLendLimit) {
            revert GlobalLendLimit();
        }

The code above shows how _deposit(...) function is implemented in the v3Vault contract, the point of interest from the pointer is the validation that is done to ensure that total supply is not above the global lend limit. However there is an oversight in this implementation, Total supply should be first converted to Asset before the validation is done, a reference to similar circumstance where totalsupply() and globalLendLimit are used can be noted from the maxDeposit(...) & maxMint(...) functions provided below. totalSupply() was first converted to asset and the resulting value was used for the validation, total supply was not used directly as they represent different variables.

   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);
        }
    }

Tools Used

Manual Review

Recommended Mitigation Steps

The correct implementation should be adjusted to the lend limit validation as provided in the code below. Total supply should be converted to asset before validation

function _deposit(address receiver, uint256 amount, bool isShare, bytes memory permitData)
        internal
        returns (uint256 assets, uint256 shares)
    {
        (, uint256 newLendExchangeRateX96) = _updateGlobalInterest();

        _resetDailyLendIncreaseLimit(newLendExchangeRateX96, false);

       ...

        _mint(receiver, shares);

---        if (totalSupply() > globalLendLimit) {
+++        if (_convertToAssets(totalSupply(), lendExchangeRateX96, Math.Rounding.Up) > globalLendLimit) {
            revert GlobalLendLimit();
        }

Assessed type

Invalid Validation

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L1025

Vulnerability details

Impact

Wrong and Inconsistent lent value Calculation in contract which would cause wrong data updated in relation to functions calling _getAvailableBalance(...) function in V3Vault contract.

Proof of Concept

 function _getAvailableBalance(uint256 debtExchangeRateX96, uint256 lendExchangeRateX96)
        internal
        view
        returns (uint256 balance, uint256 available, uint256 reserves)
    {
        balance = totalAssets();

        uint256 debt = _convertToAssets(debtSharesTotal, debtExchangeRateX96, Math.Rounding.Up);
>>>        uint256 lent = _convertToAssets(totalSupply(), lendExchangeRateX96, Math.Rounding.Down);

        reserves = balance + debt > lent ? balance + debt - lent : 0;
        available = balance > reserves ? balance - reserves : 0;
    }

The function above shows how _getAvailableBalance(...) function is implemented in the V3Vault.sol contract, as noted from the pointer lent is calculated by converting totalSupply() to Assets, the problem is that Math.Rounding.Down was used as the third parameter instead of Math.Rounding.Up which can be proven by other similar scenarios, which appeared in 9 different places in the contract, Math.Rounding.Up was used all through in similar situations except in the _getAvailableBalance(...) function as noted above. As can be confirmed from L213, L303, L314, L1134, L1225, L1250 & L1262, this mistake is totally wrong and inconsistent with lent calculation.

Tools Used

Manual Review

Recommended Mitigation Steps

The necessary adjustment should be done to the _getAvailableBalance(...) function as provided below, Math.Rounding.Up should be used instead of Math.Rounding.Down

 function _getAvailableBalance(uint256 debtExchangeRateX96, uint256 lendExchangeRateX96)
        internal
        view
        returns (uint256 balance, uint256 available, uint256 reserves)
    {
        balance = totalAssets();

        uint256 debt = _convertToAssets(debtSharesTotal, debtExchangeRateX96, Math.Rounding.Up);
---        uint256 lent = _convertToAssets(totalSupply(), lendExchangeRateX96, Math.Rounding.Down); 
+++        uint256 lent = _convertToAssets(totalSupply(), lendExchangeRateX96, Math.Rounding.Up);

        reserves = balance + debt > lent ? balance + debt - lent : 0;
        available = balance > reserves ? balance - reserves : 0;
    }

Assessed type

Error