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

11 stars 8 forks source link

Borrower can partially pay back and leave in their position a small borrowing amount/share leading to the protocol's bad debt #258

Closed c4-bot-8 closed 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/WiseLending.sol#L1088-L1155 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseLending.sol#L1161-L1198 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseLending.sol#L1204-L1238

Vulnerability details

Impact

The paybackExactAmountETH(), paybackExactAmount(), and paybackExactShares() allow borrowers to pay back their borrowing positions.

However, those functions do not check the minimum leftover borrowing amount/share. A rogue borrower can partially pay back their position and leave a small borrowing amount/share in the position. One possible objective is to disincentivize a liquidator from liquidating their position.

Since the rogue borrower's position is too small for liquidators to gain liquidation profit compared to the liquidation cost (gas), the position will not be liquidated even if it is liquidatable. This eventually leaves bad debt to the protocol.

Proof of Concept

This section will refer to the paybackExactAmountETH() only for brevity's sake. The paybackExactAmount() and paybackExactShares() should apply the same PoC concept.

As you can see, the paybackExactAmountETH() (and its dependent functions) does not check the minimum leftover borrowing amount/share.

Hence, a borrower can partially pay back and leave any borrowing amount/share in their position at will.

// https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/WiseLending.sol
function paybackExactAmountETH(
    uint256 _nftId
)
    external
    payable
    syncPool(WETH_ADDRESS)
    returns (uint256) //@audit -- No minimum leftover borrowing amount/share check
{
    uint256 maxBorrowShares = userBorrowShares[_nftId][WETH_ADDRESS];

    _validateNonZero(
        maxBorrowShares
    );

    uint256 maxPaybackAmount = paybackAmount(
        WETH_ADDRESS,
        maxBorrowShares
    );

    uint256 paybackShares = calculateBorrowShares(
        {
            _poolToken: WETH_ADDRESS,
            _amount: msg.value,
            _maxSharePrice: false
        }
    );

    _validateNonZero(
        paybackShares
    );

    uint256 refundAmount;
    uint256 requiredAmount = msg.value;

    if (msg.value > maxPaybackAmount) {

        unchecked {
            refundAmount = msg.value
                - maxPaybackAmount;
        }

        requiredAmount = requiredAmount
            - refundAmount;

        paybackShares = maxBorrowShares;
    }

    _handlePayback(
        msg.sender,
        _nftId,
        WETH_ADDRESS,
        requiredAmount,
        paybackShares
    );

    _wrapETH(
        requiredAmount
    );

    if (refundAmount > 0) {
        _sendValue(
            msg.sender,
            refundAmount
        );
    }

    return paybackShares;
}

Tools Used

Manual Review

Recommended Mitigation Steps

Do not allow a borrower to pay back their position and leave in borrowing amount/share less than the proper minimum threshold.

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 satisfactory

serial-coder commented 5 months ago

Hi @trust1995,

Issues #255 and #277 refer to the small position during the borrowing action, whereas this issue (#258) refers to the payback actions.

Fixing #255 and #277 cannot remediate this issue. Thus, this issue should be de-duplicated with #255 and #277.

Thanks for your time, sir!

trust1995 commented 5 months ago

Hi @trust1995,

Issues #255 and #277 refer to the small position during the borrowing action, whereas this issue (#258) refers to the payback actions.

Fixing #255 and #277 cannot remediate this issue. Thus, this issue should be de-duplicated with #255 and #277.

Thanks for your time, sir!

I have considered this and determined the root cause is very similar, and by the SC verdicts, if a reasonable fix of one issue would address another, they can be categorized together. Here I believe it would be very practical for the team to see both functionalities suffer from the small position flaw.

serial-coder commented 5 months ago

Hi @trust1995, Issues #255 and #277 refer to the small position during the borrowing action, whereas this issue (#258) refers to the payback actions. Fixing #255 and #277 cannot remediate this issue. Thus, this issue should be de-duplicated with #255 and #277. Thanks for your time, sir!

I have considered this and determined the root cause is very similar, and by the SC verdicts, if a reasonable fix of one issue would address another, they can be categorized together. Here I believe it would be very practical for the team to see both functionalities suffer from the small position flaw.

Thanks for the clarification, sir.