c4-bot-10 commented 3 months ago

Lines of code

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

Vulnerability details

Impact

Vulnerable VaultLogic::erc20TransferOutBidAmountToLiqudity will cause incorrect accounting of assetData or DOS liquidation.

Proof of Concept

In IsolateLogic.sol's auctions, when a bid is placed, assetData.totalBidAmount is increased with the bid amount erc20TransferInBidAmount(). When a bid is outbidded, the old bid is transferred out and in erc20TransferOutBidAmount(), assetData.totalBidAmount is subtracted by the old bidAmount.

The problem is VaultLogic::erc20TransferOutBidAmountToLiqudity will most likely cause assetData.totalBidAmount to be incorrect because it falsely assumes the final debt amount (vars.totalBorrowAmount) will equal to the total bid amount(vars.totalBidAmount).

When a loan is liquidated post-auction, due to debt compounding and due to minBidDelta requirements, vars.totalBorrowAmount and vars.totalBidAmount will differ.

//src/libraries/logic/IsolateLogic.sol
  function executeIsolateLiquidate(InputTypes.ExecuteIsolateLiquidateParams memory params) internal {
...
    for (vars.nidx = 0; vars.nidx < params.nftTokenIds.length; vars.nidx++) {
...
      // Last bid can not cover borrow amount and liquidator need pay the extra amount
      //@audit-info note: this condition means vars.totalBorrowAmount != vars.totalBidAmount
|>    if (loanData.bidAmount < vars.borrowAmount) {
        vars.extraBorrowAmounts[vars.nidx] = vars.borrowAmount - loanData.bidAmount;
      }

      // Last bid exceed borrow amount and the remain part belong to borrower
      //@audit-info note: this condition means vars.totalBorrowAmount != vars.totalBidAmount
|>    if (loanData.bidAmount > vars.borrowAmount) {
        vars.remainBidAmounts[vars.nidx] = loanData.bidAmount - vars.borrowAmount;
      }
...
      vars.totalBorrowAmount += vars.borrowAmount;
      vars.totalBidAmount += loanData.bidAmount;
...
    }
...
    // bid already in pool and now repay the borrow but need to increase liquidity
|>  VaultLogic.erc20TransferOutBidAmountToLiqudity(debtAssetData, vars.totalBorrowAmount);
...

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

In VaultLogic.erc20TransferOutBidAmountToLiqudity, vars.totalBorrowAmount will be subtracted from assetData.totalBidAmount.

//src/libraries/logic/VaultLogic.sol
  function erc20TransferOutBidAmountToLiqudity(DataTypes.AssetData storage assetData, uint amount) internal {
    //@audit amount is likely not a bid amount when called from IsolateLogic::executeIsolateLiquidate
|>  assetData.totalBidAmout -= amount;
    assetData.availableLiquidity += amount;
  }

(https://github.com/code-423n4/2024-07-benddao/blob/117ef61967d4b318fc65170061c9577e674fffa1/src/libraries/logic/VaultLogic.sol#L497)

(1) vars.totalBorrowAmount > vars.totalBidAmount; assetData.totalBidAmout -= amount might revert due to underflow, liquidation will fail.

(2) vars.totalBorrowAmount < vars.totalBidAmount; assetData.totalBidAmount will be inflated.

Tools Used

Manual

Recommended Mitigation Steps

Consider change erc20TransferOutBidAmountToLiqudity with an extra input variable totalBidAmount.

  function erc20TransferOutBidAmountToLiqudity(DataTypes.AssetData storage assetData,uint bidAmount, uint amount) internal {
    assetData.totalBidAmout -= bidAmount;
    assetData.availableLiquidity += amount;
  }