Potential risk that a pool may become inactive after the initial check

[c4-bot-2]

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/FeeManager/FeeManager.sol#L730-L814

Vulnerability details

Impact

If a pool becomes inactive after the initial check, the function may proceed with the payback operation, but the subsequent operations involving that pool may fail or behave unexpectedly. For example, if the receiving token pool becomes inactive, the function may not be able to correctly calculate or transfer the receiving token amount, potentially leading to loss of funds or other unintended consequences. The severity of this issue can be considered High. The potential loss of funds, inconsistent state, and stuck funds can have significant financial implications and undermine the reliability and trustworthiness of the contract. Additionally, the inability to handle the scenario where a pool becomes inactive during the execution of the function can lead to unexpected behavior and potential vulnerabilities.

Proof of Concept

The paybackBadDebtForToken function allows users to pay back the bad debt of a position by providing a payback token and receiving a corresponding amount of another token as a reward. However, there is a potential vulnerability related to the assumption that the pools for both the payback token and the receiving token are active. The function checks if the pools for the _paybackToken and _receivingToken are active by verifying if their total deposit shares are non-zero:

   if (WISE_LENDING.getTotalDepositShares(_receivingToken) == 0) {
       revert PoolNotActive();
    }

   if (WISE_LENDING.getTotalDepositShares(_paybackToken) == 0) {
       revert PoolNotActive();
   }

However, this check is performed only once at the beginning of the function. If either of the pools becomes inactive after this initial check, the function may proceed with the payback operation, leading to inconsistencies or unexpected behavior.

Proof of Concept and Vulnerability Details: Suppose the _receivingToken pool is active when the function is called, but it becomes inactive after the initial check. The function will proceed with the payback operation, and when it tries to calculate the receiving amount using the getReceivingToken function, it may encounter issues or unexpected behavior due to the pool being inactive. Similarly, if the _paybackToken pool becomes inactive after the initial check, the subsequent operations involving that pool, such as transferring the payback amount, may fail or lead to inconsistent state.

Tools Used

Manual

Recommended Mitigation Steps

Perform the pool activity check not only at the beginning of the function but also immediately before any operations involving those pools. This way, you can ensure that the pools are still active before performing critical operations.

   function paybackBadDebtForToken(
       uint256 _nftId,
       address _paybackToken,
       address _receivingToken,
       uint256 _shares
   )
       external
       returns (
           uint256 paybackAmount,
           uint256 receivingAmount
      )
   {
       updatePositionCurrentBadDebt(_nftId);

       if (badDebtPosition[_nftId] == 0) {
           return (0, 0);
       }

       // Check if pools are active
       if (WISE_LENDING.getTotalDepositShares(_receivingToken) == 0) {
           revert PoolNotActive();
       }

       if (WISE_LENDING.getTotalDepositShares(_paybackToken) == 0) {
           revert PoolNotActive();
        }

        paybackAmount = WISE_LENDING.paybackAmount(_paybackToken, _shares);

        // Check if the payback token pool is still active before proceeding
       if (WISE_LENDING.getTotalDepositShares(_paybackToken) == 0) {
             revert PoolNotActive();
        }

        WISE_LENDING.corePaybackFeeManager(_paybackToken, _nftId, 
    paybackAmount, _shares);

        _updateUserBadDebt(_nftId);

        // Check if the receiving token pool is still active before 
     proceeding
      ✅  if (WISE_LENDING.getTotalDepositShares(_receivingToken) == 0) {
            revert PoolNotActive();
       }

        receivingAmount = getReceivingToken(_paybackToken, _receivingToken, 
     paybackAmount);

       _decreaseFeeTokens(_receivingToken, receivingAmount);

       _safeTransferFrom(_paybackToken, msg.sender, address(WISE_LENDING), 
    paybackAmount);
        _safeTransfer(_receivingToken, msg.sender, receivingAmount);

        emit PayedBackBadDebt(
            _nftId,
            msg.sender,
            _paybackToken,
            _receivingToken,
            paybackAmount,
            block.timestamp
       );
    }

In the modified code, additional checks for the pool activity are performed immediately before the critical operations involving those pools, such as transferring tokens or updating balances. This ensures that the pools are still active at the time of these operations, mitigating the potential vulnerability.

Assessed type

Other

[vm06007]

How can a pool become inactive during the transaction? Please provide a test where it can be proven that there can be a condition that pool suddenly becomes inactive without any transaction going before user proceed with paybackBadDebtForToken call

[vonMangoldt]

the totalDepositShares from the initial ghost_amount can never be destroyed so it is not possible to have an inactive pool by this check after it has been deployed. Feel free to provide poc testfile though

[c4-pre-sort]

GalloDaSballo marked the issue as insufficient quality report

[vm06007]

based on comment from @vonMangoldt I can agree and dismiss this issue. mark it as invalid.

[c4-judge]

trust1995 marked the issue as unsatisfactory: Insufficient proof