BBLiquidation::_updateBorrowAndCollateralShare Liquidator can avoid having his bonus reduced when position is close to bad-debt

[hats-bug-reporter[bot]]

Github username: @CergyK Twitter username: -- Submission hash (on-chain): 0x2cfe926149c7050450f63055e2978066a8d53d048aaf708ffe9e9b96eb5297a9 Severity: medium

Description: Description: During the calculation of the computeLiquidationFactor, a condition is enforced on collateralPartInAsset and borrowPartWithBonus, to ensure that a liquidator cannot bypass the bad-debt which should be handled separately. The condition enforced is: https://github.com/hats-finance/Tapioca--Lending-Engine--0x5bee198f5b060eecd86b299fdbea6b0c07c728dd/blob/8920782db6044643fd0c682f58ef37f7e59f99b1/contracts/markets/bigBang/BBLiquidation.sol#L200

However this is insufficient to ensure that a liquidator passes a maxBorrowAmount big enough and that liquidation is handled under this condition: https://github.com/hats-finance/Tapioca--Lending-Engine--0x5bee198f5b060eecd86b299fdbea6b0c07c728dd/blob/8920782db6044643fd0c682f58ef37f7e59f99b1/contracts/markets/bigBang/BBLiquidation.sol#L220-L233

Indeed in the case where collateralPartInAsset > userTotalBorrowAmount but collateralPartInAsset < userTotalBorrowAmount + liquidationBonus, the liquidation bonus should be reduced (according to the above snippet).

To avoid this the liquidator can pass in a maxBorrowAmount such as borrowPartWithBonus is reduced here: https://github.com/hats-finance/Tapioca--Lending-Engine--0x5bee198f5b060eecd86b299fdbea6b0c07c728dd/blob/8920782db6044643fd0c682f58ef37f7e59f99b1/contracts/markets/bigBang/BBLiquidation.sol#L203

In that case the second condition is used: https://github.com/hats-finance/Tapioca--Lending-Engine--0x5bee198f5b060eecd86b299fdbea6b0c07c728dd/blob/8920782db6044643fd0c682f58ef37f7e59f99b1/contracts/markets/bigBang/BBLiquidation.sol#L234-L238

and the borrower receives a bigger share of the liquidation bonus, and can leave the protocol with pure bad-debt (uncollateralized debt).

Attack scenario:

Params:
        - liquidationBonus = 10%

Scenario:
  - Alice has a position which has:
                `collateralPartInAsset = userTotalBorrowAmount + 1`

  - Bob liquidates the position, by using `maxBorrowAmount` == collateralPartInAsset / (1 + liquidationBonus), such as when liquidation bonus is added, `borrowPartWithBonus < collateralPartInAsset`.

  - Bob receives the full liquidation bonus, and all of the Alice's collateral is extracted out of the position. However 10% of Alice's debt remains unpaid, and is fully uncollateralized.

Recommendation: Check that total user debt with hypothetical liquidation bonus is not above collateralPartInAsset

https://github.com/hats-finance/Tapioca--Lending-Engine--0x5bee198f5b060eecd86b299fdbea6b0c07c728dd/blob/8920782db6044643fd0c682f58ef37f7e59f99b1/contracts/markets/bigBang/BBLiquidation.sol#L233-L239

} else {
+       uint totalUserBorrowWithBonus = userTotalBorrowAmount + (userTotalBorrowAmount * liquidationBonusAmount) / FEE_PRECISION;
+       if (collateralPartInAsset < totalUserBorrowWithBonus) revert BadDebt();
          collateralShare =
                    yieldBox.toShare(collateralId, (borrowPartWithBonus * _exchangeRate) / EXCHANGE_RATE_PRECISION, false);
         if (collateralShare > userCollateralShare[user]) {
                    revert NotEnoughCollateral();
         }
}

[CergyK]

Typo in the first sentence:

During the calculation of the computeLiquidationFactor

Should be

During _updateBorrowAndCollateralShare

[cryptotechmaker]

@CergyK , I think something is off. If the code enters the 2nd branch where collateralPartInAsset > borrowPartWithBonus, the used collateral is equal to borrowPartWithBonus while the borrow part used is borrowPart which, given the scenario should be the entire user position. So, if borrowPart = 1000, collateralInAsset is 1100, at this point, after updating the values, it should be borrowPart = 0, collateral = 0. Isn't this right?

The 2 conditions you added seem redundant because we also have this one

 if (liquidationBonusAmount > 0) {
            borrowPartWithBonus = borrowPartWithBonus + (borrowPartWithBonus * liquidationBonusAmount) / FEE_PRECISION;
 }

which happens after sanitizing the maxBorrowPart condition.

Let's take the following scenario: borrowed 1000 collateralInAsset 1100 maxBorrowPart 900

=> borrowPart = 900 borrowPartWithBonus = 990

  `collateralPartInAsset < borrowPartWithBonus` => `1100 < 990` false => goes with the 2nd branch
    .. else branch..
     `collateralShare = 990 in shares`

     At the end, collateral paid is 990, borrow part repayed is 900.. so it's basically 900 + 10% bonus

  User is left with :
   borrowed 10
   collateralInAsset 110

[CergyK]

Let's take the following scenario: borrowed 1000 collateralInAsset 1100 maxBorrowPart 900

With a 10% liquidation bonus, these values should not trigger the bug indeed.

If we consider

• borrowed 1050
• collateralInAsset 1100
• maxBorrowPart 1000

Is where the problem arises, the repay amount (1000) with liquidation bonus would be 1000 * 1.1 = 1100 So the whole collateral is taken.

The debt of 50 remaining at the end has zero collateral associated with it

[cryptotechmaker]

Kinda hard to reach this point as liquidationCollateralizationRate would need to be >95% and collateralizationRate even higher.

[CergyK]

Kinda hard to reach this point as liquidationCollateralizationRate would need to be >95% and collateralizationRate even higher.

It's not very likely, but still a bit more likely than a position reaching bad debt. It can happen with any liquidationCollateralizationRate.

Also hence the medium severity due to likelihood since impact is unbounded loss of funds

[maarcweiss]

Even if the likelihood is very very small, the scenario is not impossible. We think it is one the edge of Low-Medium, but we will mark it as medium