Revert Lend - DanielArmstrong's results

Lines of code

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

Vulnerability details

Impact

Since daily increase limit is set much larger than the intention, it is possible to cause a catastrophic exploit to the protocol.

Proof of Concept

V3Vault.sol#_resetDailyLendIncreaseLimit function is the following.

    function _resetDailyLendIncreaseLimit(uint256 newLendExchangeRateX96, bool force) internal {
        // daily lend limit reset handling
        uint256 time = block.timestamp / 1 days;
        if (force || time > dailyLendIncreaseLimitLastReset) {
1250:       uint256 lendIncreaseLimit = _convertToAssets(totalSupply(), newLendExchangeRateX96, Math.Rounding.Up)
                * (Q32 + MAX_DAILY_LEND_INCREASE_X32) / Q32;
            dailyLendIncreaseLimitLeft =
                dailyLendIncreaseLimitMin > lendIncreaseLimit ? dailyLendIncreaseLimitMin : lendIncreaseLimit;
            dailyLendIncreaseLimitLastReset = time;
        }
    }

L1250 calculates the maximum increase of lending amount during a day to 110% of the present total shares. This is 11 times greater than the intended maximum increase of 10%.

Thus, there is a possibility of causing a catastrophic exploit to the protocol by malicious attacker's attacks.

V3Vault.sol#_resetDailyDebtIncreaseLimit function also has the same issue.

Tools Used

Manual Review

Recommended Mitigation Steps

Modify V3Vault.sol#_resetDailyLendIncreaseLimit function as follows.

    function _resetDailyLendIncreaseLimit(uint256 newLendExchangeRateX96, bool force) internal {
        // daily lend limit reset handling
        uint256 time = block.timestamp / 1 days;
        if (force || time > dailyLendIncreaseLimitLastReset) {
            uint256 lendIncreaseLimit = _convertToAssets(totalSupply(), newLendExchangeRateX96, Math.Rounding.Up)
--              * (Q32 + MAX_DAILY_LEND_INCREASE_X32) / Q32;
++              * MAX_DAILY_LEND_INCREASE_X32 / Q32;
            dailyLendIncreaseLimitLeft =
                dailyLendIncreaseLimitMin > lendIncreaseLimit ? dailyLendIncreaseLimitMin : lendIncreaseLimit;
            dailyLendIncreaseLimitLastReset = time;
        }
    }

In the same way, modify V3Vault.sol#_resetDailyDebtIncreaseLimit function as follows.

    function _resetDailyDebtIncreaseLimit(uint256 newLendExchangeRateX96, bool force) internal {
        // daily debt limit reset handling
        uint256 time = block.timestamp / 1 days;
        if (force || time > dailyDebtIncreaseLimitLastReset) {
            uint256 debtIncreaseLimit = _convertToAssets(totalSupply(), newLendExchangeRateX96, Math.Rounding.Up)
--              * (Q32 + MAX_DAILY_DEBT_INCREASE_X32) / Q32;
++              * MAX_DAILY_DEBT_INCREASE_X32 / Q32;
            dailyDebtIncreaseLimitLeft =
                dailyDebtIncreaseLimitMin > debtIncreaseLimit ? dailyDebtIncreaseLimitMin : debtIncreaseLimit;
            dailyDebtIncreaseLimitLastReset = time;
        }
    }

Assessed type

Math

Lines of code

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

Vulnerability details

Impact

First withdraw or repay cannot increase daily available deposit or borrow amount. This results in decreasing the system's incentives. When a user withdraw or repay funds just after new date started, it increases dailyLendIncreaseLimitLeft or dailyDebtIncreaseLimitLeft but it has no effect. That's why it resets them when deposit or borrow. So daily available deposit or borrow amount cannot be increased.

Proof of Concept

V3Vault.sol#_withdraw function is as follows.

    function _withdraw(address receiver, address owner, uint256 amount, bool isShare)
        internal
        returns (uint256 assets, uint256 shares)
    {
        (uint256 newDebtExchangeRateX96, uint256 newLendExchangeRateX96) = _updateGlobalInterest();

        ...

        // when amounts are withdrawn - they may be deposited again
949     dailyLendIncreaseLimitLeft += assets;

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

As you can see above, on L949 it increases dailyLendIncreaseLimitLeft without considering current date.
And V3Vault.sol#_deposit function is as follows.

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

883     _resetDailyLendIncreaseLimit(newLendExchangeRateX96, false);

        ...

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

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

On L883, it resets dailyLendIncreaseLimitLeft if available through _resetDailyLendIncreaseLimit function.

    function _resetDailyLendIncreaseLimit(uint256 newLendExchangeRateX96, bool force) internal {
        // daily lend limit reset handling
        uint256 time = block.timestamp / 1 days;
        if (force || time > dailyLendIncreaseLimitLastReset) {
            uint256 lendIncreaseLimit = _convertToAssets(totalSupply(), newLendExchangeRateX96, Math.Rounding.Up)
                * (Q32 + MAX_DAILY_LEND_INCREASE_X32) / Q32;
            dailyLendIncreaseLimitLeft =
                dailyLendIncreaseLimitMin > lendIncreaseLimit ? dailyLendIncreaseLimitMin : lendIncreaseLimit;
            dailyLendIncreaseLimitLastReset = time;
        }
    }

As we can see, if new date starts, it resets dailyLendIncreaseLimitLeft.

So first withdraw cannot increase daily available deposit amount.

This problem occurs in repay and borrow as same way.

Tools Used

Manual Review

Recommended Mitigation Steps

We have to modify V3Vault.sol#_withdraw, _repay functions as follows.

    function _withdraw(address receiver, address owner, uint256 amount, bool isShare)
        internal
        returns (uint256 assets, uint256 shares)
    {
        (uint256 newDebtExchangeRateX96, uint256 newLendExchangeRateX96) = _updateGlobalInterest();
        
+       _resetDailyLendIncreaseLimit(newLendExchangeRateX96, false);

        ...

        // when amounts are withdrawn - they may be deposited again
        dailyLendIncreaseLimitLeft += assets;

        emit Withdraw(msg.sender, receiver, owner, assets, shares);
    }
    ...
    function _repay(uint256 tokenId, uint256 amount, bool isShare, bytes memory permitData) internal {
        (uint256 newDebtExchangeRateX96, uint256 newLendExchangeRateX96) = _updateGlobalInterest();

+       _resetDailyDebtIncreaseLimit(newLendExchangeRateX96, false);

        ...

        // when amounts are repayed - they maybe borrowed again
        dailyDebtIncreaseLimitLeft += assets;

        ...

        emit Repay(tokenId, msg.sender, owner, assets, shares);
    }

Assessed type

Error

Lines of code

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

Vulnerability details

Impact

When resetting limit values, it resets dailyLendIncreaseLimit and dailyDebyIncreaseLimit forcefully. So daily amounts of lend and debt can exceed limits at that day.

Proof of Concept

V3Vault.sol#setLimits function which resets limits is as follows.

    function setLimits(
        uint256 _minLoanSize,
        uint256 _globalLendLimit,
        uint256 _globalDebtLimit,
        uint256 _dailyLendIncreaseLimitMin,
        uint256 _dailyDebtIncreaseLimitMin
    ) external {
        if (msg.sender != emergencyAdmin && msg.sender != owner()) {
            revert Unauthorized();
        }

        minLoanSize = _minLoanSize;
        globalLendLimit = _globalLendLimit;
        globalDebtLimit = _globalDebtLimit;
        dailyLendIncreaseLimitMin = _dailyLendIncreaseLimitMin;
        dailyDebtIncreaseLimitMin = _dailyDebtIncreaseLimitMin;

        (, uint256 newLendExchangeRateX96) = _updateGlobalInterest();

        // force reset daily limits with new values
827     _resetDailyLendIncreaseLimit(newLendExchangeRateX96, true);
828     _resetDailyDebtIncreaseLimit(newLendExchangeRateX96, true);

        emit SetLimits(
            _minLoanSize, _globalLendLimit, _globalDebtLimit, _dailyLendIncreaseLimitMin, _dailyDebtIncreaseLimitMin
        );
    }

As we can see, on L827, 828 it resets dailyLendIncreaseLimit and dailyDebtIncreaseLimit forcefully.

Tools Used

Manual Review

Recommended Mitigation Steps

We have modify V3Vault.sol#setLimits as follows.

    function setLimits(
        uint256 _minLoanSize,
        uint256 _globalLendLimit,
        uint256 _globalDebtLimit,
        uint256 _dailyLendIncreaseLimitMin,
        uint256 _dailyDebtIncreaseLimitMin
    ) external {
        ...

        (, uint256 newLendExchangeRateX96) = _updateGlobalInterest();

-       // force reset daily limits with new values
-       _resetDailyLendIncreaseLimit(newLendExchangeRateX96, true);
-       _resetDailyDebtIncreaseLimit(newLendExchangeRateX96, true);

        emit SetLimits(
            _minLoanSize, _globalLendLimit, _globalDebtLimit, _dailyLendIncreaseLimitMin, _dailyDebtIncreaseLimitMin
        );
    }

Assessed type

Error