Total bad debts are overstated and can affect access and distribution of benefits

[c4-bot-8]

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseLending.sol#L1306
https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseCore.sol#L665-L667
https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseSecurity/WiseSecurity.sol#L423-L435
https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/FeeManager/FeeManager.sol#L494-L508
https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/FeeManager/FeeManagerHelper.sol#L89-L100

Vulnerability details

Impact

The total debt is overstated, which can result in fees earned under the protocol not being reutilized

Proof of Concept

When a position has more than 100% debt, it can be liquidated via the
method liquidatePartiallyFromTokens to liquidate the position. The amount to be liquidated is fine as long as it does not exceed the maximum liquidatable shares. When liquidating, the total bad debt incurred and the bad debt of the position will be recorded separately.The code is as follows:
https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseSecurity/WiseSecurity.sol#L405-L436

    function checkBadDebtLiquidation(
        uint256 _nftId
    )
        external
        onlyWiseLending
    {
        uint256 bareCollateral = overallETHCollateralsBare(
            _nftId
        );

        uint256 totalBorrow = overallETHBorrowBare(
            _nftId
        );

        if (totalBorrow < bareCollateral) {
            return;
        }

        unchecked {
            uint256 diff = totalBorrow
                - bareCollateral; // @aduit The total bad debt amount of the position

            FEE_MANAGER.increaseTotalBadDebtLiquidation(
                diff
            ); // @audit The total bad debt keeps increasing

            FEE_MANAGER.setBadDebtUserLiquidation(
                _nftId,
                diff
            );
        }
    }

The problem occurs when the total bad debt is recorded. We can see the logic of the bad debt, which is calculated in full.
For example, the bad debt value of a position bad debt is 5ETH.
The first liquidation is not liquidated, the repayment of 1ETH, then the total bad debt record is 4ETH, the bad debt of the position record is 4ETH. the second user made another repayment of 1ETH, then the bad debt of the position record is 3ETH, the total bad debt continues to increase, the record is 7ETH.In fact, the total bad debt should only be 3ETH.the remaining bad debt even if the call to the Even if the updatePositionCurrentBadDebt method is called to clear the bad debt, it can only clear 3ETH of the position's record. the total bad debt is reduced by 3ETH, and the total bad debt is still left at 4ETH. the total bad debt can't be completely cleared, and the affected methods are:
claimFeesBeneficial
claimWiseFees
claimWiseFeesBulk
will result in the agreement fees not being able to be withdrawn and will only be in locked in the agreement.

Also this makes it easy to launch a DDos attack, as long as a position is liquidated 2 times this problem will occur. Bad users get liquidation rewards normally. Costs are very low.

Tools Used

Manual Reveiw

Recommended Mitigation Steps

There are 2 ways to solve this problem:

1. the first is simpler is that a position must be liquidated all at once
2. the second method, when recording the total amount of bad debt to determine the position bad debt decreased or increased, and then keep the total amount and the position of data synchronization.

Assessed type

Math

[c4-pre-sort]

GalloDaSballo marked the issue as sufficient quality report

[GalloDaSballo]

A coded POC would have been better, worth checking

[c4-pre-sort]

GalloDaSballo marked the issue as duplicate of #243

[trust1995]

This explanation is exceptionally hard to follow. I have the right to close it because it:

1. Is submitted as High
2. Lacks coded / step by step POC

[c4-judge]

trust1995 marked the issue as unsatisfactory:
Insufficient quality

[wangxx2026]

hello @trust1995, To better express the problems found, sometimes I write POC, sometimes I post the code directly. I found that some other wardens have added links directly to their methods, which are concise and selected. I'll try to do the same, hopefully I can express the problem I found better. After this issue was raised, I checked a few times to check if my expression was clear, wrote out the flow of the problem, pasted in the core method, gave a simple and easy to understand example, and hopefully expressed my meaning clearly. I searched for all possible impacts, worrying about missing impacts. I spent a long time reading the code, auditing the project, I want to express better, I want to submit the issue can be "selected for report". When I see comments that POC is better, I tell myself to write POC in the future, I try to find the problem, I try to improve my expression, I hope the person who sees the issue can read it with minimum effort, I know that a bad issue will make the judge feel bad, I try to refine the issue, I try to become a better warden, I'm always trying to improve my expression. I've been trying to improve my presentation skills. I wrote out the problem function with a link, I think it is more clearly expressed and conducive to reading, if posting the code, is more conducive to reading, then I will definitely post all the relevant code in the future. As a warden, no one tells you how to express better, only explore by yourself little by little. I would like to add issue, I hope it can meet the requirements.

We can look at the flow of the liquidation, which will first call the liquidatePartiallyFromTokens method with the following code:
https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseLending.sol#L1250-L1309

    function liquidatePartiallyFromTokens(
        uint256 _nftId,
        uint256 _nftIdLiquidator,
        address _paybackToken,
        address _receiveToken,
        uint256 _shareAmountToPay
    )
        external
        syncPool(_paybackToken)
        syncPool(_receiveToken)
        returns (uint256)
    {
        CoreLiquidationStruct memory data;

        data.nftId = _nftId;
        data.nftIdLiquidator = _nftIdLiquidator;

        data.caller = msg.sender;

        data.tokenToPayback = _paybackToken;
        data.tokenToRecieve = _receiveToken;
        data.shareAmountToPay = _shareAmountToPay;

        data.maxFeeETH = WISE_SECURITY.maxFeeETH();
        data.baseRewardLiquidation = WISE_SECURITY.baseRewardLiquidation();

        (
            data.lendTokens,
            data.borrowTokens
        ) = _prepareAssociatedTokens(
            _nftId,
            _receiveToken,
            _paybackToken
        );

        data.paybackAmount = paybackAmount(
            _paybackToken,
            _shareAmountToPay
        ); // @audit Convert the shares to be liquidated into quantities to be paid out

        _checkPositionLocked(
            _nftId,
            msg.sender
        );  // @audit check whether the position is locked

        _checkLiquidatorNft(
            _nftId,
            _nftIdLiquidator
        ); // @audit check whether __nftIdLiquidator can liquidate _nftId

        WISE_SECURITY.checksLiquidation(
            _nftId,
            _paybackToken,
            _shareAmountToPay
        ); // @audit check whether the liquidation share exceeds the maximum liquidation share

        return _coreLiquidation(
            data
        );

We can see that this method mainly carries out the following operations:

1. Convert the shares to be liquidated into quantities to be paid out
2. check whether the position is locked
3. check whether _nftIdLiquidator can liquidate _nftId
4. check whether the liquidation share exceeds the maximum liquidable share
5. Call _coreLiquidation

The fourth step calls the checksLiquidation method to check that the maximum share is not exceeded.

    function checksLiquidation(
        uint256 _nftIdLiquidate,
        address _tokenToPayback,
        uint256 _shareAmountToPay
    )
        external
        view
    {
...
        // @audit Checks that the maximum liquidation share is not exceeded
        checkMaxShares(
            _nftIdLiquidate,
            _tokenToPayback,
            borrowETHTotal,
            unweightedCollateralETH,
            _shareAmountToPay
        );
    }

The checkMaxShares function code is as follows:
https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseSecurity/WiseSecurityHelper.sol#L876-L900

    function checkMaxShares(
        uint256 _nftId,
        address _tokenToPayback,
        uint256 _borrowETHTotal,
        uint256 _unweightedCollateralETH,
        uint256 _shareAmountToPay
    )
        public
        view
    {
...
        if (_shareAmountToPay <= maxShares) { // @audit Not greater than the maximum liquidation amount is fine, otherwise it will REVERT
            return;
        }

        revert TooManyShares();
    }

The fifth step is the core of liquidation,_coreLiquidation method code is as follows.
https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseCore.sol#L625-L686

    function _coreLiquidation(
        CoreLiquidationStruct memory _data
    )
        internal
        returns (uint256 receiveAmount)
    {
        _validateNonZero(
            _data.paybackAmount
        );
        // @audit Calculate the amount liquidated as a percentage of the assets of the liquidation position
        uint256 collateralPercentage = WISE_SECURITY.calculateWishPercentage(
            _data.nftId,
            _data.tokenToRecieve,
            WISE_ORACLE.getTokensInETH(
                _data.tokenToPayback,
                _data.paybackAmount
            ),
            _data.maxFeeETH,
            _data.baseRewardLiquidation
        );

        _validateParameter(
            collateralPercentage,
            PRECISION_FACTOR_E18
        );
        // @audit Update the number of pools after liquidation
        _corePayback(
            _data.nftId,
            _data.tokenToPayback,
            _data.paybackAmount,
            _data.shareAmountToPay
        );
        // @audit Calculate the amount of assets that the liquidator will get
        receiveAmount = _calculateReceiveAmount(
            _data.nftId,
            _data.nftIdLiquidator,
            _data.tokenToRecieve,
            collateralPercentage
        );
        // @audit Check for bad debts
        WISE_SECURITY.checkBadDebtLiquidation(
            _data.nftId
        );
        // @audit Security check on lendTokens and borrowTokens
        _curveSecurityChecks(
            _data.lendTokens,
            _data.borrowTokens
        );
        // @audit Pull the tokens that need to be returned from the liquidator
        _safeTransferFrom(
            _data.tokenToPayback,
            _data.caller,
            address(this),
            _data.paybackAmount
        );
        // @audit Send the number of assets that the liquidator can obtain
        _safeTransfer(
            _data.tokenToRecieve,
            _data.caller,
            receiveAmount
        );
    }

_coreLiquidation method mainly for the following operations:
1.Calculate the amount liquidated as a percentage of the assets of the liquidation position
2.Update the number of pools after liquidation
3.Calculate the amount of assets that the liquidator will get
4.Check for bad debts
5.Security check on lendTokens and borrowTokens
6.Pull the tokens that need to be returned from the liquidator
7.Send the number of assets that the liquidator can obtain

The problem arises in step 4, calling checkBadDebtLiquidation to check for bad debt
https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseSecurity/WiseSecurity.sol#L405-L436

    function checkBadDebtLiquidation(
        uint256 _nftId
    )
        external
        onlyWiseLending
    {
...
        unchecked {
            uint256 diff = totalBorrow
                - bareCollateral; // @audit Calculate the total bad debt amount

            FEE_MANAGER.increaseTotalBadDebtLiquidation(
                diff
            ); // @audit Record the total amount of bad debt incurred

            FEE_MANAGER.setBadDebtUserLiquidation(
                _nftId,
                diff
            ); // @audit Record the amount of bad debt generated by the position
        }
    }

A bad debt arises when the amount borrowed is greater than the amount of the asset. Then record the bad debt separately
1.Record the total amount of bad debts by increasingTotalBadDebtLiquidation
Then call _increaseTotalBadDebt to record the total bad debt in the
FeeManager.totalBadDebtETH variable
https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/FeeManager/FeeManagerHelper.sol#L89-L100

    function _increaseTotalBadDebt(
        uint256 _amount
    )
        internal
    {
        totalBadDebtETH += _amount; // Record the total bad debt
...
    }

2.Record the position amount through setBadDebtUserLiquidation
Then call _setBadDebtPosition to record the bad debt position in FeeManager.badDebtPosition variable.
https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/FeeManager/FeeManagerHelper.sol#L77-L84

    function _setBadDebtPosition(
        uint256 _nftId,
        uint256 _amount
    )
        internal
    {
        badDebtPosition[_nftId] = _amount;
    }

The total bad debt amount recorded at this point is the amount of bad debt incurred.
If a user generates 5ETH bad debt, it will be recorded as 5ETH, if this position is liquidated in 2 times, the bad debt in the first time is 5ETH, the bad debt in the second liquidation is 5ETH, the total bad debt will be recorded as 10ETH, while the actual bad debt is only 5ETH.

Then the 2 methods paybackBadDebtForToken, paybackBadDebtNoReward can be called to clear the bad debt.
We use the paybackBadDebtForToken method as an example, which is implemented as follows

    function paybackBadDebtForToken(
        uint256 _nftId,
        address _paybackToken,
        address _receivingToken,
        uint256 _shares
    )
        external
        returns (
            uint256 paybackAmount,
            uint256 receivingAmount
        )
    {
        updatePositionCurrentBadDebt(
            _nftId
        ); // @audit Update the position price information and call _updateUserBadDebt to update the bad debt situation. 
...

        WISE_LENDING.corePaybackFeeManager(
            _paybackToken,
            _nftId,
            paybackAmount,
            _shares
        ); // @audit Update the pool information after the position pays off the bad debt.

        _updateUserBadDebt(
            _nftId
        );  // @audit Update bad debt
...
    }

The paybackBadDebtForToken method mainly implements the following functions:
1.Update the position price information and call _updateUserBadDebt to update the bad debt situation.
2.Update the pool information after the position pays off the bad debt.
3.Update bad debt

The implementation of _updateUserBadDebt is as follows:

    function _updateUserBadDebt(
        uint256 _nftId
    )
        internal
    {

...
        uint256 currentBadDebt = badDebtPosition[
            _nftId
        ]; // @audit The current bad debt in the position

        if (currentBorrowETH < currentCollateralBareETH) {

            _eraseBadDebtUser(
                _nftId
            ); // @audit If there are no more bad debts, the position bad debt is set to

            _decreaseTotalBadDebt(
                currentBadDebt
            ); // @audit The total bad debt can be reduced by up to the current bad debt in the position


            emit UpdateBadDebtPosition(
                _nftId,
                0,
                block.timestamp
            );

            return;
        }

        unchecked {
            uint256 newBadDebt = currentBorrowETH
                - currentCollateralBareETH; // @audit New bad debt

            _setBadDebtPosition(
                _nftId,
                newBadDebt
            ); // @audit Sets the position's new bad debt

            newBadDebt > currentBadDebt
                ? _increaseTotalBadDebt(newBadDebt - currentBadDebt)
                : _decreaseTotalBadDebt(currentBadDebt - newBadDebt); // @audit Increase or decrease the corresponding total bad debt
...
        }
    }

This method mainly updates the bad debt, we can see that the total bad debt can only be reduced by the bad debt in the position

If a position is liquidated more than once, the total bad debt is greater than the bad debt of the position. And there is no way to clear the total bad debt to 0 again.

The inability to clear the total bad debt to zero will cause the following functions to be affected
claimFeesBeneficial will result in fees not being captured

    function claimFeesBeneficial(
        address _feeToken,
        uint256 _amount
    )
        external
    {
        address caller = msg.sender;

        if (totalBadDebtETH > 0) { // The total bad debt must be equal to 0 in order to execute
            revert ExistingBadDebt();
        }
...
    }

claimWiseFees Rewards cannot be distributed

    function claimWiseFees(
        address _poolToken
    )
        public
    {
...
        if (totalBadDebtETH == 0) { // @audit Rewards cannot be distributed

            tokenAmount = _distributeIncentives(
                tokenAmount,
                _poolToken,
                underlyingTokenAddress
            );
        }
...
    }

claimWiseFeesBulk is a batch execution of claimWiseFees, which will result in rewards not being distributed

    function claimWiseFeesBulk()
        external
    {
        uint256 i;
        uint256 l = getPoolTokenAddressesLength();

        while (i < l) {
            claimWiseFees(
                poolTokenAddresses[i]
            ); // Bulk execution of claimWiseFees

            unchecked {
                ++i;
            }
        }

        emit ClaimedFeesWiseBulk(
            block.timestamp
        );
    }

[trust1995]

I appreciate the resubmission, however I must maintain that the original section was not sufficient for High severity. I must leave it unsatisfactory, but I'm sure you will improve your submission writing in future contests!

[c4-judge]

trust1995 marked the issue as duplicate of #74

[stalinMacias]

@trust1995 Marking this report as a duplicate of #74 wouldn't cause it to be included in the rewards distribution?

[trust1995]

No, because it is marked as unsatisfactory.

[wangxx2026]

@trust1995 Thank you for your reply, I can understand not meeting High's criteria, I guess there is no compromise to give me a partial reward? It took me almost 2 weeks to find out about this issue and this is the only issue I have had a chance with this program. can this issue be handled as medium or QA? Since being unemployed as a warden and earning very little, it makes me happy to earn 100$ at a time. Thanks again.