code-423n4 / 2024-02-wise-lending-findings

11 stars 8 forks source link

The protocol allows borrowing small positions that can create bad debt #255

Open c4-bot-8 opened 5 months ago

c4-bot-8 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseSecurity/WiseSecurity.sol#L306-L330 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseCore.sol#L260-L263 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseSecurity/WiseSecurity.sol#L1100-L1102 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseSecurity/WiseSecurity.sol#L1104-L1106 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseSecurity/WiseSecurity.sol#L237-L270

Vulnerability details

Impact

The Wise Lending protocol allows users to borrow small positions. Even if the protocol has a minimum deposit (collateral) amount check to mitigate the small borrowing position from creating bad debt, this protection can be bypassed.

With a small borrowing position, there is no incentive for a liquidator to liquidate the position, as the liquidation profit may not cover the liquidation cost (gas). As a result, small liquidable positions will not be liquidated, leaving bad debt to the protocol.

Proof of Concept

The protocol allows users to borrow small positions since no minimum borrowing amount is checked in the WiseSecurity::checksBorrow().

// FILE: https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/WiseSecurity/WiseSecurity.sol
function checksBorrow(
    uint256 _nftId,
    address _caller,
    address _poolToken
)
    external
    view
    returns (bool specialCase) //@audit -- No minimum borrowing amount check
{
    _checkPoolCondition(
        _poolToken
    );

    checkTokenAllowed(
        _poolToken
    );

    if (WISE_LENDING.verifiedIsolationPool(_caller) == true) {
        return true;
    }

    if (WISE_LENDING.positionLocked(_nftId) == true) {
        return true;
    }
}

Even if the protocol has a minimum deposit (collateral) amount check in the WiseCore::_checkDeposit() to mitigate the small borrowing position from creating bad debt, this protection can be easily bypassed.

The WiseCore::_checkMinDepositValue() is a core function that checks a minimum deposit (collateral) amount. By default, this deposit amount check would be overridden (disabled). Even though this deposit amount check will be enabled, this protection can be bypassed by withdrawing the deposited fund later since there is no minimum withdrawal amount check in the WiseSecurity::checksWithdraw().

    // FILE: https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/WiseCore.sol
    function _checkDeposit(
        uint256 _nftId,
        address _caller,
        address _poolToken,
        uint256 _amount
    )
        internal
        view
    {

        if (WISE_ORACLE.chainLinkIsDead(_poolToken) == true) {
            revert DeadOracle();
        }

        _checkAllowDeposit(
            _nftId,
            _caller
        );

        _checkPositionLocked(
            _nftId,
            _caller
        );

@1      WISE_SECURITY.checkPoolWithMinDeposit( //@audit -- Even if there is a minimum deposit amount check, this protection can be bypassed
@1          _poolToken,
@1          _amount
@1      );

        _checkMaxDepositValue(
            _poolToken,
            _amount
        );
    }

    // FILE: https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/WiseSecurity/WiseSecurity.sol
    function _checkMinDepositValue(
        address _token,
        uint256 _amount
    )
        private
        view
        returns (bool)
    {
@2      if (minDepositEthValue == ONE_WEI) { //@audit -- By default, the minimum deposit amount check would be overridden (disabled)
@2          return true;
@2      }

@3      if (_getTokensInEth(_token, _amount) < minDepositEthValue) { //@audit -- Even though the minimum deposit amount check will be enabled, this protection can be bypassed by withdrawing the deposited fund later
@3          revert DepositAmountTooSmall();
@3      }

        return true;
    }

As you can see, there is no minimum withdrawal amount check in the WiseSecurity::checksWithdraw(). Hence, a user can deposit collateral at or above the minimum deposit amount (i.e., minDepositEthValue) and then withdraw the deposited fund to be under the minDepositEthValue. Later, they can borrow a small amount with small collateral.

With a small borrowing position (and small collateral), there is no incentive for a liquidator to liquidate the position, as the liquidation profit may not cover the liquidation cost (gas). As a result, small liquidable positions will not be liquidated, leaving bad debt to the protocol.

// FILE: https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/WiseSecurity/WiseSecurity.sol
function checksWithdraw(
    uint256 _nftId,
    address _caller,
    address _poolToken
)
    external
    view
    returns (bool specialCase) //@audit -- No minimum withdrawal amount check
{
    if (_checkBlacklisted(_poolToken) == true) {

        if (overallETHBorrowBare(_nftId) > 0) {
            revert OpenBorrowPosition();
        }

        return true;
    }

    if (WISE_LENDING.verifiedIsolationPool(_caller) == true) {
        return true;
    }

    if (WISE_LENDING.positionLocked(_nftId) == true) {
        return true;
    }

    if (_isUncollateralized(_nftId, _poolToken) == true) {
        return true;
    }

    if (WISE_LENDING.getPositionBorrowTokenLength(_nftId) == 0) {
        return true;
    }
}

Tools Used

Manual Review

Recommended Mitigation Steps

Implement the minimum borrowing amount check to limit the minimum size of borrowing positions.

Assessed type

Other

c4-pre-sort commented 5 months ago

GalloDaSballo marked the issue as duplicate of #277

c4-pre-sort commented 5 months ago

GalloDaSballo marked the issue as sufficient quality report

c4-judge commented 5 months ago

trust1995 marked the issue as selected for report

trust1995 commented 5 months ago

The bug class is imho valid as either liquidator will liquidate at a loss or protocol will be losing money to protect from bad debt over time.

serial-coder commented 5 months ago

Hi @trust1995,

Did this issue miss the satisfactory tag?

Thanks!

trust1995 commented 5 months ago

Hi @trust1995,

Did this issue miss the satisfactory tag?

Thanks!

I believe selected-for-report qualifies as satisfactory implicitly.

serial-coder commented 5 months ago

Hi @trust1995, Did this issue miss the satisfactory tag? Thanks!

I believe selected-for-report qualifies as satisfactory implicitly.

Thanks for the quick response, sir.

thebrittfactor commented 3 months ago

For transparency and per conversation with the sponsors, see here for the Wise Lending team's mitigation.