User are forced to borrow again in order to unlock their NFTs from `IsolateLending.sol`

[c4-bot-9]

Lines of code

https://github.com/code-423n4/2024-07-benddao/blob/main/src/libraries/logic/IsolateLogic.sol#L356-L357

Vulnerability details

Description

Isolated Margin Lending the BendDAO protocol will calculate the heath factor based on a single collateral (one NFT), the liquidation of collateral will be achieved by selling the NFT through an auction mechanism.

However, the borrower who gets liquidated (his NFT is in the auction state) still able to repay his loan and close the auction, by triggering IsolateLiquidation.sol#isolateRedeem().

Each asset has a redemption threshold redeemThreshold to define the minimum debt repayment amount by the borrower

Filse: IsolateLogic.sol#executeIsolateRedeem()

vars.redeemAmounts[vars.nidx] = vars.borrowAmount.percentMul(nftAssetData.redeemThreshold);

The redeemThreshold is updateable from Configurator.sol#setAssetAuctionParams() with a max value of 10_000

In case the user calls IsolateLiquidation.sol#isolateRedeem() and nftAssetData.redeemThreshold is 10_000 (MAX_REDEEM_THRESHOLD) this will be set scaledAmount to zero

File: IsolateLogic.sol#executeIsolateRedeem()

       loanData.scaledAmount -= vars.amountScaled;

This means the debt is fully paid However, 1- the loanData.loanStatus will still be in active state Constants.LOAN_STATUS_ACTIVE 2- the erc721TokenData.LockerAddr is not updated to address(0),

So, the user is not able to withdraw his NFT from BVault.sol or even update the loan state and LockerAddr values through IsolateLending.sol#isolateRepay() due to this check

Note: the only way the user can unlock his NFT is by borrowing again, this will increase loanData.scaledAmount again to greater than zero, and then he needs to repay that loan IsolateLending.sol#isolateRepay() to update the erc721TokenData.LockerAddr to address(0).

and this will expose the user to the unnecessary risk of being liquidated again.

Impact

Users are forced to borrow again to unlock their NFTs After Isolate Redeem

Proof of Concept

Foundry PoC: Please copy the following POC in TestIntIsolateRedeem.t.sol

  function test_Withdraw_After_Full_Redeem_Should_Revert() public {
    TestCaseLocalVars memory testVars;

    tsHEVM.startPrank(tsPoolAdmin);
    tsConfigurator.setAssetAuctionParams(tsCommonPoolId, address(tsBAYC), 10000, 500, 2000, 1 days);
    tsHEVM.stopPrank();
    // deposit
    prepareUSDT(tsDepositor1);
    uint256[] memory tokenIds = prepareIsolateBAYC(tsBorrower1);

    // borrow
    prepareBorrow(tsBorrower1, address(tsBAYC), tokenIds, address(tsUSDT));

    // make some interest
    advanceTimes(365 days);

    // drop down nft price
    actionSetNftPrice(address(tsBAYC), 5000);

    // auction
    prepareAuction(tsLiquidator1, address(tsBAYC), tokenIds, address(tsUSDT));

    uint256[] memory redeemAmounts = new uint256[](tokenIds.length);
    testVars.loanDataBefore = getIsolateLoanData(tsCommonPoolId, address(tsBAYC), tokenIds);
    for (uint256 i = 0; i < tokenIds.length; i++) {
      testVars.totalBidAmount += testVars.loanDataBefore[i].bidAmount;
      testVars.totalBidFine += testVars.loanDataBefore[i].bidFine;
      testVars.totalRedeemAmount += testVars.loanDataBefore[i].redeemAmount;
      redeemAmounts[i] = testVars.loanDataBefore[i].redeemAmount;
    }

    testVars.poolBalanceBefore = tsUSDT.balanceOf(address(tsPoolManager));
    testVars.walletBalanceBefore1 = tsUSDT.balanceOf(address(tsLiquidator1));
    testVars.walletBalanceBefore2 = tsUSDT.balanceOf(address(tsBorrower1));

    // redeem
    tsBorrower1.approveERC20(address(tsUSDT), type(uint256).max);
    tsBorrower1.isolateRedeem(tsCommonPoolId, address(tsBAYC), tokenIds, address(tsUSDT), redeemAmounts);
    testVars.txAuctionTimestamp = block.timestamp;

    // check results
    testVars.loanDataAfter = getIsolateLoanData(tsCommonPoolId, address(tsBAYC), tokenIds);
    for (uint256 i = 0; i < tokenIds.length; i++) {
      assertEq(testVars.loanDataAfter[i].bidStartTimestamp, 0, 'bidStartTimestamp');
      assertEq(testVars.loanDataAfter[i].firstBidder, address(0), 'firstBidder');
      assertEq(testVars.loanDataAfter[i].lastBidder, address(0), 'lastBidder');
      assertEq(testVars.loanDataAfter[i].bidAmount, 0, 'bidAmount');
      assertEq(testVars.loanDataAfter[i].scaledAmount, 0, 'scaledAmount');
    }
    tsHEVM.expectRevert(bytes(Errors.ASSET_ALREADY_LOCKED_IN_USE));
    tsBorrower1.withdrawERC721(
      tsCommonPoolId,
      address(tsBAYC),
      tokenIds,
      Constants.SUPPLY_MODE_ISOLATE,
      address(tsBorrower1),
      address(tsBorrower1)
    );
  }

Tools Used

Manual Review - Foundry

Recommended Mitigation Steps

In case loanData.scaledAmount == 0 call VaultLogic.erc721SetTokenLockerAddr(nftAssetData, params.nftTokenIds[vars.nidx], address(0));

Assessed type

Other

[c4-judge]

MarioPoneder marked the issue as primary issue

[c4-judge]

MarioPoneder marked the issue as selected for report

[c4-judge]

MarioPoneder marked the issue as satisfactory

[thorseldon]

Fixed at https://github.com/BendDAO/bend-v2/commit/e535e2af895ab04a44218cce8e4512bc6e897583.