Incorrect bad debt accounting can lead to a state where the claimFeesBeneficial function is permanently bricked and no new incentives can be distributed, potentially locking pending and future protocol fees in the FeeManager contract

criticalCode4rena

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/WiseSecurity/WiseSecurity.sol#L419-L434 https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/FeeManager/FeeManager.sol#L689-L699 https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/FeeManager/FeeManager.sol#L663-L670

Vulnerability details

Bug Description

Protocol fees can be collected from the WiseLending contract and sent to the FeeManager contract via the permissionless FeeManager::claimWiseFees function. During this call, incentives will only be distributed for incentive owners if totalBadDebtETH (global bad debt) is equal to 0:

FeeManager::claimWiseFees

solidity📋
663:        if (totalBadDebtETH == 0) { // @audit: incentives only distributed if there is no global bad debt
664:
665:            tokenAmount = _distributeIncentives( // @audit: distirbutes incentives for `incentive owners` via `gatheredIncentiveToken` mapping
666:                tokenAmount,
667:                _poolToken,
668:                underlyingTokenAddress
669:            );
670:        }

The fees sent to the FeeManager are then able to be claimed by beneficials via the FeeManager::claimFeesBeneficial function or by incentive owners via the FeeManager::claimIncentives function (if incentives have been distributed to the owners):

FeeManager::claimIncentives

solidity📋
284:    function claimIncentives(
285:        address _feeToken
286:    )
287:        public
288:    {
289:        uint256 amount = gatheredIncentiveToken[msg.sender][_feeToken]; // @audit: mapping incremented in _distributeIncentives function
290:
291:        if (amount == 0) {
292:            revert NoIncentive();
293:        }

However, beneficials are only able to claim fees if there is currently no global bad debt in the system (totalBadDebtETH == 0).

FeeManager::claimFeesBeneficial

solidity📋
689:    function claimFeesBeneficial(
690:        address _feeToken,
691:        uint256 _amount
692:    )
693:        external
694:    {
695:        address caller = msg.sender;
696:
697:        if (totalBadDebtETH > 0) { // @audit: can't claim fees when there is bad debt
698:            revert ExistingBadDebt();
699:        }

Below I will explain how the bad debt accounting logic used during partial liquidations can result in a state where totalBadDebtETH is permanently greater than 0. When this occurs, beneficials will no longer be able to claim fees via the FeeManager::claimFeesBeneficial function and new incentives will no longer be distributed when fees are permissionlessly collected via the FeeManager::claimWiseFees function.

When a position is partially liquidated, the WiseSecurity::checkBadDebtLiquidation function is executed to check if the position has created bad debt, i.e. if the position's overall borrow value is greater than the overall (unweighted) collateral value. If the post liquidation state of the position created bad debt, then the bad debt is recorded in a global and position-specific state:

WiseSecurity::checkBadDebtLiquidation

solidity📋
405:    function checkBadDebtLiquidation(
406:        uint256 _nftId
407:    )
408:        external
409:        onlyWiseLending
410:    {
411:        uint256 bareCollateral = overallETHCollateralsBare(
412:            _nftId
413:        );
414:
415:        uint256 totalBorrow = overallETHBorrowBare(
416:            _nftId
417:        );
418:
419:        if (totalBorrow < bareCollateral) { // @audit: LTV < 100%
420:            return;
421:        }
422:
423:        unchecked {
424:            uint256 diff = totalBorrow
425:                - bareCollateral;
426:
427:            FEE_MANAGER.increaseTotalBadDebtLiquidation( // @audit: global state, totalBadDebtETH += diff
428:                diff
429:            );
430:
431:            FEE_MANAGER.setBadDebtUserLiquidation( // @audit: position state, badDebtPosition[_nftId] = diff
432:                _nftId,
433:                diff
434:            );
435:        }
436:    }

FeeManagerHeper.sol

solidity📋
77:    function _setBadDebtPosition(
78:        uint256 _nftId,
79:        uint256 _amount
80:    )
81:        internal
82:    {
83:        badDebtPosition[_nftId] = _amount; // @audit: position bad debt set
84:    }
85:
86:    /**
87:     * @dev Internal increase function for global bad debt amount.
88:     */
89:    function _increaseTotalBadDebt(
90:        uint256 _amount
91:    )
92:        internal
93:    {
94:        totalBadDebtETH += _amount; // @audit: total bad debt incremented

As we can see above, the method by which the global and position's state is updated is not consistent (total debt increases, but position's debt is set to recent debt). Since liquidations can be partial, a position with bad debt can undergo multiple partial liquidations and each time the totalBadDebtETH will be incremented. However, the badDebtPosition for the position will only be updated with the most recent bad debt that was recorded during the last partial liquidation. Note that due to the condition on line 419 of WiseSecurity::checkBadDebtLiquidation, the badDebtPosition will be reset to 0 when totalBorrow == bareCollateral (LTV == 100%). However, in this case, any previously recorded bad debt for the position will not be deducted from the totalBadDebtETH. Lets consider two examples:

Scenario 1: Due to a market crash, a position's LTV goes above 100%. The position gets partially liquidated, incrementing totalBadDebtETH by x (bad debt from 1st liquidation) and setting badDebtPosition[_nftId] to x. The position gets partially liquidated again, this time incrementing totalBadDebtETH by y (bad debt from 2nd liquidation) and setting badDebtPosition[_nftId] to y. The resulting state:

totalBadDebtETH == x + y
badDebtPosition[_nftId] == y

Scenario 2: Due to a market crash, a position's LTV goes above 100%. The position gets partially liquidated, incrementing totalBadDebtETH by x and setting badDebtPosition[_nftId] to x. The position gets partially liquidated again, but this time the totalBorrow is equal to bareCollateral (LTV == 100%) and thus no bad debt is created. Due to the condition on line 419, totalBadDebtETH will be incremented by 0, but badDebtPosition[_nftId] will be reset to 0. The resulting state:

totalBadDebtETH == x
badDebtPosition[_nftId] == 0

Note that scenario 1 is more likely to occur since scenario 2 requires the additional partial liquidation to result in an LTV of exactly 100% for the position.

As we can see, partial liquidations can lead to totalBadDebtETH being artificially inflated with respect to the actual bad debt created by a position.

When bad debt is created, it is able to be paid back via the FeeManager::paybackBadDebtForToken or FeeManager::paybackBadDebtNoReward functions. However, the maximum amount of bad debt that can be deducted during these calls is capped at the bad debt recorded for the position specified (badDebtPosition[_nftId]). Therefore, the excess "fake" bad debt can not be deducted from totalBadDebtETH, resulting in totalBadDebtETH being permanently greater than 0.

Below is the logic that deducts the bad debt created by a position when it is paid off via one of the payback functions mentioned above:

FeeManagerHelper::_updateUserBadDebt

solidity📋
170:        unchecked {
171:            uint256 newBadDebt = currentBorrowETH
172:                - currentCollateralBareETH;
173:
174:            _setBadDebtPosition( // @audit: badDebtPosition[_nftId] = newBadDebt
175:                _nftId,
176:                newBadDebt
177:            );
178:
179:            newBadDebt > currentBadDebt // @audit: totalBadDebtETH updated with respect to change in badDebtPosition
180:                ? _increaseTotalBadDebt(newBadDebt - currentBadDebt)
181:                : _decreaseTotalBadDebt(currentBadDebt - newBadDebt);

The above code is invoked in the FeeManagerHelper::updatePositionCurrentBadDebt function, which is in turn invoked during both of the payback functions previously mentioned. You will notice that the above code properly takes into account the change in the bad debt of the position in question. I.e. if the badDebtPosition[_nftId] decreased (after being paid back), then the totalBadDebtETH will decrease as well. Therefore, the totalBadDebtETH can only be deducted by at most the current bad debt of a position. Returning to the previous example in scenario 1, this means that totalBadDebtETH would remain equal to x, since only y amount of bad debt can be paid back.

Impact

In the event a position creates bad debt, partial liquidations of that position can lead to the global totalBadDebtETH state variable being artificially inflated. This additional "fake debt" can not be deducted from the global state when the actual bad debt of the position is paid back. Thus, the FeeManager::claimFeesBeneficial function will be permanently DOS-ed, preventing any beneficials from claiming fees in the FeeManager contract. Additionally, no new incentives are able to be distributed to incentive owners in this state. However, protocol fees can still be collected in this state via the permissionless FeeManager::claimWiseFees function, and since incentive owners and beneficials are the only entities able to claim these fees, this can lead to fees being permanently locked in the FeeManager contract.

Justification for Medium Severity:

Although not directly affecting end users, the function of claiming beneficial fees and distributing new incentives will be permanently bricked. To make matters worse, anyone can continue to collect fees via the permissionless FeeManager::claimWiseFees function, which will essentially "burn" any pending or future fees by locking them in the FeeManager (assuming all previously gathered incentives have been claimed). This value is therefore leaked from the protocol every time additional fees are collected in this state.

Once this state is reached, any pending or future fees should ideally be left in the WiseLending contract, providing value back to the users instead of allowing that value to be unnecessarily "burned". However, the permissionless nature of the FeeManager::claimWiseFees function allows bad actors to further grief the protocol during this state by continuing to collect fees.

Note that once this state is reached, and Wise Lending is made aware of the implications, all fees (for all pools) can be set to 0 by the master address. This would ensure that no future fees are sent to the FeeManager. However, this does not stop pending fees from being collected. Additionally, a true decentralized system (such as a DAO) would likely have some latency between proposing such a change (decreasing fee value) and executing that change. Therefore, any fees distributed during that period can be collected.

Proof of Concept

Place the following test in the contracts/ directory and run with forge test --match-path contracts/BadDebtTest.t.sol:

solidity📋
// SPDX-License-Identifier: -- WISE --

pragma solidity =0.8.24;

import "./WiseLendingBaseDeployment.t.sol";

contract BadDebtTest is BaseDeploymentTest {
    address borrower = address(0x01010101);
    address lender = address(0x02020202);

    uint256 depositAmountETH = 10e18; // 10 ether
    uint256 depositAmountToken = 10; // 10 ether
    uint256 borrowAmount = 5e18; // 5 ether

    uint256 nftIdLiquidator; // nftId of lender
    uint256 nftIdLiquidatee; // nftId of borrower

    uint256 debtShares;

    function _setupIndividualTest() internal override {
        _deployNewWiseLending(false);

        // set token value for simple calculations
        MOCK_CHAINLINK_2.setValue(1 ether); // 1 token == 1 ETH
        assertEq(MOCK_CHAINLINK_2.latestAnswer(), MOCK_CHAINLINK_ETH_ETH.latestAnswer());
        vm.stopPrank();
        
        // fund lender and borrower
        vm.deal(lender, depositAmountETH);
        deal(address(MOCK_WETH), lender, depositAmountETH);
        deal(address(MOCK_ERC20_2), borrower, depositAmountToken * 2);
    }


    function testScenario1() public {
        // --- scenario is set up --- //
        _setUpScenario();

        // --- shortfall event/crash creates bad debt, position partially liquidated logging bad debt --- //
        _marketCrashCreatesBadDebt();

        // --- borrower gets partially liquidated again --- //
        vm.prank(lender);

        LENDING_INSTANCE.liquidatePartiallyFromTokens(
            nftIdLiquidatee,
            nftIdLiquidator, 
            address(MOCK_WETH),
            address(MOCK_ERC20_2),
            debtShares * 2e16 / 1e18
        );

        // --- global bad det increases again, but user bad debt is set to current bad debt created --- // 
        uint256 newTotalBadDebt = FEE_MANAGER_INSTANCE.totalBadDebtETH();
        uint256 newUserBadDebt = FEE_MANAGER_INSTANCE.badDebtPosition(nftIdLiquidatee);
        
        assertGt(newUserBadDebt, 0); // userBadDebt reset to new bad debt, newUserBadDebt == current_bad_debt_created
        assertGt(newTotalBadDebt, newUserBadDebt); // global bad debt incremented again
        // newTotalBadDebt = old_global_bad_debt + current_bad_debt_created
        
        // --- user bad debt is paid off, but global bad is only partially paid off (remainder is fake debt) --- // 
        _tryToPayBackGlobalDebt();

        // --- protocol fees can no longer be claimed since totalBadDebtETH will remain > 0 --- // 
        vm.expectRevert(bytes4(keccak256("ExistingBadDebt()")));
        FEE_MANAGER_INSTANCE.claimFeesBeneficial(address(0), 0);
    }

    function testScenario2() public {
        // --- scenario is set up --- // 
        _setUpScenario();

        // --- shortfall event/crash creates bad debt, position partially liquidated logging bad debt --- //
        _marketCrashCreatesBadDebt();
        
        // --- Position manipulated so second partial liquidation results in totalBorrow == bareCollateral --- //
        // borrower adds collateral
        vm.prank(borrower);

        LENDING_INSTANCE.solelyDeposit(
            nftIdLiquidatee, 
            address(MOCK_ERC20_2), 
            6
        );

        // borrower gets partially liquidated again
        vm.prank(lender);

        LENDING_INSTANCE.liquidatePartiallyFromTokens(
            nftIdLiquidatee,
            nftIdLiquidator, 
            address(MOCK_WETH),
            address(MOCK_ERC20_2),
            debtShares * 2e16 / 1e18
        );
        
        uint256 collateral = SECURITY_INSTANCE.overallETHCollateralsBare(nftIdLiquidatee);
        uint256 debt = SECURITY_INSTANCE.overallETHBorrowBare(nftIdLiquidatee);
        assertEq(collateral, debt); // LTV == 100% exactly

        // --- global bad debt is unchanged, while user bad debt is reset to 0 --- // 
        uint256 newTotalBadDebt = FEE_MANAGER_INSTANCE.totalBadDebtETH();
        uint256 newUserBadDebt = FEE_MANAGER_INSTANCE.badDebtPosition(nftIdLiquidatee);

        assertEq(newUserBadDebt, 0); // user bad debt reset to 0
        assertGt(newTotalBadDebt, 0); // global bad debt stays the same (fake debt)

        // --- attempts to pay back fake global debt result in a noop, totalBadDebtETH still > 0 --- // 
        uint256 paybackShares = _tryToPayBackGlobalDebt();
        
        assertEq(LENDING_INSTANCE.userBorrowShares(nftIdLiquidatee, address(MOCK_WETH)), paybackShares); // no shares were paid back

        // --- protocol fees can no longer be claimed since totalBadDebtETH will remain > 0 --- //
        vm.expectRevert(bytes4(keccak256("ExistingBadDebt()")));
        FEE_MANAGER_INSTANCE.claimFeesBeneficial(address(0), 0);
    }

    function _setUpScenario() internal {
        // lender supplies ETH
        vm.startPrank(lender);

        nftIdLiquidator = POSITION_NFTS_INSTANCE.mintPosition();

        LENDING_INSTANCE.depositExactAmountETH{value: depositAmountETH}(nftIdLiquidator);

        vm.stopPrank();

        // borrower supplies collateral token and borrows ETH
        vm.startPrank(borrower);

        MOCK_ERC20_2.approve(address(LENDING_INSTANCE), depositAmountToken * 2);

        nftIdLiquidatee = POSITION_NFTS_INSTANCE.mintPosition();
        
        LENDING_INSTANCE.solelyDeposit( // supply collateral
            nftIdLiquidatee, 
            address(MOCK_ERC20_2), 
            depositAmountToken
        );

        debtShares = LENDING_INSTANCE.borrowExactAmountETH(nftIdLiquidatee, borrowAmount); // borrow ETH

        vm.stopPrank();
    }

    function _marketCrashCreatesBadDebt() internal {
        // shortfall event/crash occurs
        vm.prank(MOCK_DEPLOYER);

        MOCK_CHAINLINK_2.setValue(0.3 ether);

        // borrower gets partially liquidated
        vm.startPrank(lender);

        MOCK_WETH.approve(address(LENDING_INSTANCE), depositAmountETH);

        LENDING_INSTANCE.liquidatePartiallyFromTokens(
            nftIdLiquidatee,
            nftIdLiquidator, 
            address(MOCK_WETH),
            address(MOCK_ERC20_2),
            debtShares * 2e16 / 1e18 + 1 
        );

        vm.stopPrank();

        // global and user bad debt is increased
        uint256 totalBadDebt = FEE_MANAGER_INSTANCE.totalBadDebtETH();
        uint256 userBadDebt = FEE_MANAGER_INSTANCE.badDebtPosition(nftIdLiquidatee);

        assertGt(totalBadDebt, 0); 
        assertGt(userBadDebt, 0);
        assertEq(totalBadDebt, userBadDebt); // user bad debt and global bad debt are the same
    }

    function _tryToPayBackGlobalDebt() internal returns (uint256 paybackShares) {
        // lender attempts to pay back global debt
        paybackShares = LENDING_INSTANCE.userBorrowShares(nftIdLiquidatee, address(MOCK_WETH));
        uint256 paybackAmount = LENDING_INSTANCE.paybackAmount(address(MOCK_WETH), paybackShares);

        vm.startPrank(lender);

        MOCK_WETH.approve(address(FEE_MANAGER_INSTANCE), paybackAmount);
        
        FEE_MANAGER_INSTANCE.paybackBadDebtNoReward(
            nftIdLiquidatee, 
            address(MOCK_WETH), 
            paybackShares
        );

        vm.stopPrank();

        // global bad debt and user bad debt updated
        uint256 finalTotalBadDebt = FEE_MANAGER_INSTANCE.totalBadDebtETH();
        uint256 finalUserBadDebt = FEE_MANAGER_INSTANCE.badDebtPosition(nftIdLiquidatee);

        assertEq(finalUserBadDebt, 0); // user has no more bad debt, all paid off
        assertGt(finalTotalBadDebt, 0); // protocol still thinks there is bad debt
    }
}

Tools Used

Manual

Recommended Mitigation Steps

I would recommend updating totalBadDebtETH with the difference of the previous and new bad debt of a position in the WiseSecurity::checkBadDebtLiquidation function, similar to how it is done in the FeeManagerHelper::_updateUserBadDebt internal function.

Example implementation:

diff📋
diff --git a/./WiseSecurity/WiseSecurity.sol b/./WiseSecurity/WiseSecurity.sol
index d2cfb24..75a34e8 100644
--- a/./WiseSecurity/WiseSecurity.sol
+++ b/./WiseSecurity/WiseSecurity.sol
@@ -424,14 +424,22 @@ contract WiseSecurity is WiseSecurityHelper, ApprovalHelper {
             uint256 diff = totalBorrow
                 - bareCollateral;

-            FEE_MANAGER.increaseTotalBadDebtLiquidation(
-                diff
-            );
+            uint256 currentBadDebt = FEE_MANAGER.badDebtPosition(_nftId);

             FEE_MANAGER.setBadDebtUserLiquidation(
                 _nftId,
                 diff
             );
+
+            if (diff > currentBadDebt) {
+                FEE_MANAGER.increaseTotalBadDebtLiquidation(
+                    diff - currentBadDebt
+                );
+            } else {
+                FEE_MANAGER.decreaseTotalBadDebtLiquidation(
+                    currentBadDebt - diff
+                );
+            }
         }
     }

Assessed type

Other