code-423n4 / 2024-07-benddao-findings

9 stars 6 forks source link

executeIsolateLiquidate() totalBidAmout is not accounting correctly #48

Closed c4-bot-10 closed 3 months ago

c4-bot-10 commented 3 months ago

Lines of code

https://github.com/code-423n4/2024-07-benddao/blob/117ef61967d4b318fc65170061c9577e674fffa1/src/libraries/logic/IsolateLogic.sol#L464

Vulnerability details

Vulnerability details

When execute isolateLiquidate(), we will accounting totalBidAmout

  function executeIsolateLiquidate(InputTypes.ExecuteIsolateLiquidateParams memory params) internal {
...
    for (vars.nidx = 0; vars.nidx < params.nftTokenIds.length; vars.nidx++) {
...
      // transfer remain amount to borrower
      if (vars.remainBidAmounts[vars.nidx] > 0) {
        VaultLogic.erc20TransferOutBidAmount(debtAssetData, tokenData.owner, vars.remainBidAmounts[vars.nidx]);
      }
...
      vars.totalBorrowAmount += vars.borrowAmount;
      vars.totalBidAmount += loanData.bidAmount;
      vars.totalExtraAmount += vars.extraBorrowAmounts[vars.nidx];

      // delete the loan data at final
      delete poolData.loanLookup[params.nftAsset][params.nftTokenIds[vars.nidx]];
    }

    require(
      (vars.totalBidAmount + vars.totalExtraAmount) >= vars.totalBorrowAmount,
      Errors.ISOLATE_LOAN_BORROW_AMOUNT_NOT_COVER
    );

    // update interest rate according latest borrow amount (utilizaton)
    InterestLogic.updateInterestRates(poolData, debtAssetData, (vars.totalBorrowAmount + vars.totalExtraAmount), 0);

    // bid already in pool and now repay the borrow but need to increase liquidity
@>  VaultLogic.erc20TransferOutBidAmountToLiqudity(debtAssetData, vars.totalBorrowAmount);
  function erc20TransferOutBidAmountToLiqudity(DataTypes.AssetData storage assetData, uint amount) internal {
@>  assetData.totalBidAmout -= amount;
    assetData.availableLiquidity += amount;
  }

At the end of the method, we will use erc20TransferOutBidAmountToLiqudity(vars.totalBorrowAmount) to reduce the totalBidAmoutassetData.totalBidAmout -= vars.totalBorrowAmount

Using vars.totalBorrowAmount may be incorrect Over time (borrowIndex increase), vars.totalBorrowAmount may be greater than vars.totalBidAmount This will result in assetData.totalBidAmout being subtracted too much

Impact

assetData.totalBidAmout was incorrectly subtracted too much , which could lead to subsequent methods underflow

Recommended Mitigation

  1. when have remainBidAmounts, don't use erc20TransferOutBidAmount() , it will reduce totalBidAmout
  2. erc20TransferOutBidAmountToLiqudity() use totalBidAmount

    
    function executeIsolateLiquidate(InputTypes.ExecuteIsolateLiquidateParams memory params) internal {
    ...
      // transfer remain amount to borrower
      if (vars.remainBidAmounts[vars.nidx] > 0) {
    -       VaultLogic.erc20TransferOutBidAmount(debtAssetData, tokenData.owner, vars.remainBidAmounts[vars.nidx]);
    +       assetData.underlyingAsset.safeTransfer(tokenData.owner,vars.remainBidAmounts[vars.nidx]);
      }
    ...
    
    // bid already in pool and now repay the borrow but need to increase liquidity
    -   VaultLogic.erc20TransferOutBidAmountToLiqudity(debtAssetData, vars.totalBorrowAmount);
    +   VaultLogic.erc20TransferOutBidAmountToLiqudity(debtAssetData, vars.totalBidAmount);


## Assessed type

Context
c4-judge commented 3 months ago

MarioPoneder marked the issue as primary issue

c4-judge commented 3 months ago

MarioPoneder marked the issue as duplicate of #27

c4-judge commented 3 months ago

MarioPoneder changed the severity to 3 (High Risk)

c4-judge commented 3 months ago

MarioPoneder marked the issue as satisfactory