The `paybackBadDebtForToken` function is also vulnerable to reentrancy attacks.

[c4-bot-10]

Lines of code

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

Vulnerability details

Impact

An attacker could potentially drain the contract's funds by repeatedly calling the function before the badDebtPosition mapping is updated. Here's how the reentrancy attack could work:

1. The attacker creates a malicious contract that calls paybackBadDebtForToken from within its fallback function.
2. The attacker calls paybackBadDebtForToken with their malicious contract as the caller.
3. Inside the paybackBadDebtForToken function, the contract transfers the receivingAmount tokens to the attacker's malicious contract before updating the badDebtPosition mapping.
4. The malicious contract's fallback function is triggered by the token transfer, and it recursively calls paybackBadDebtForToken again.
5. Since the badDebtPosition mapping hasn't been updated yet, the attacker can repeatedly receive the receivingAmount tokens. This attack can continue until the contract's funds are depleted or the gas limit is reached. The severity of this vulnerability is considered high because it could lead to a complete loss of funds for the contract. An attacker could potentially drain all the tokens held by the contract, resulting in a significant financial

Proof of Concept

The paybackBadDebtForToken function is also vulnerable to reentrancy attacks. An attacker could potentially call this function recursively before the badDebtPosition mapping is updated, leading to multiple paybacks of the same bad debt. True or false, explain in details how this works, what is the impact ? show relevant code sections and explain proof of concept and vulnerability details. Use code to explain how it can be mitigated. The paybackBadDebtForToken function is indeed vulnerable to a reentrancy attack. Here's a breakdown of the vulnerability and how it can be exploited: 1.The function updates the badDebtPosition mapping after transferring the receivingAmount and the paybackAmount tokens. This means that an attacker's malicious contract can re-enter the function before the badDebtPosition mapping is updated.

2. The attacker's malicious contract would call paybackBadDebtForToken and receive the receivingAmount token.

3. Before the badDebtPosition mapping is updated, the attacker's contract would recursively call paybackBadDebtForToken again, exploiting the fact that the badDebtPosition mapping has not been updated yet. 4.This recursive call would allow the attacker to receive additional receivingAmount tokens before the badDebtPosition mapping is updated, effectively draining the contract's tokens. Here's the relevant code section that illustrates the vulnerability:

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

         // ... (code omitted for brevity)
   
      receivingAmount = getReceivingToken(
          _paybackToken,
          _receivingToken,
          paybackAmount
      );
   
         _decreaseFeeTokens(
           _receivingToken,
           receivingAmount
      );
   
      _safeTransferFrom(
          _paybackToken,
          msg.sender,
           address(WISE_LENDING),
           paybackAmount
     );
   
      _safeTransfer(
            _receivingToken,
            msg.sender,
            receivingAmount
      );
   
        // <-- Attacker's contract can re-enter here before the mapping is 
      updated
   
       _updateUserBadDebt(
             _nftId
       );
   
      // ... (code omitted for brevity)

   }

Tools Used

Manual

Recommended Mitigation Steps

Follow the "Checks-Effects-Interactions" pattern and update the badDebtPosition mapping before transferring any tokens. Here's how you can modify the paybackBadDebtForToken function:

   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);
        }

       // ... (code omitted for brevity)

       paybackAmount = WISE_LENDING.paybackAmount(
           _paybackToken,
           _shares
       );

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

       _updateUserBadDebt(
            _nftId
      );

       receivingAmount = getReceivingToken(
           _paybackToken,
           _receivingToken,
           paybackAmount
       );

       _decreaseFeeTokens(
           _receivingToken,
            receivingAmount
       );

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

       _safeTransfer(
           _receivingToken,
           msg.sender,
           receivingAmount
       );

       // ... (code omitted for brevity)
   }

In this modified version, the badDebtPosition mapping is updated before transferring any tokens, preventing the reentrancy attack. Additionally, a check is added to ensure that the badDebtPosition is not zero before proceeding with the payback process. By following the "Checks-Effects-Interactions" pattern and updating the critical state variables before any external interactions, you can effectively mitigate the reentrancy vulnerability in the paybackBadDebtForToken function.

Assessed type

Other

[Foon256]

On corePaybackFeeManager() there syncPool() modifier is called in which _syncPoolBeforeCodeExecution() is called and in this function the function _checkReentrancy() is triggered.

Beside that there are not token sent to the user in corePaybackFeeManager(). At the end after _updateUserBadDebt() the incentive token are sent to the user as you can see in the function.

[c4-pre-sort]

GalloDaSballo marked the issue as insufficient quality report

[c4-judge]

trust1995 marked the issue as unsatisfactory: Insufficient proof