Users can make collaterals to be constantly non-liquidatable during recovery mode due to Flashloan attack on liquidation

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-badger/blob/main/packages/contracts/contracts/LiquidationLibrary.sol#L93 https://github.com/code-423n4/2023-10-badger/blob/main/packages/contracts/contracts/CdpManagerStorage.sol#L53 https://github.com/code-423n4/2023-10-badger/blob/main/packages/contracts/contracts/BorrowerOperations.sol#L874 https://github.com/code-423n4/2023-10-badger/blob/main/packages/contracts/contracts/BorrowerOperations.sol#L891 https://github.com/code-423n4/2023-10-badger/blob/main/packages/contracts/contracts/BorrowerOperations.sol#L385

Vulnerability details

Impact

Users can reset the grace period that's about ending continually, preventing it from ever ending and leaving collaterals non-liquidatable during recovery mode

Proof of Concept

Liquation operations enforce a grace period before liquidating, it checks this grace period has exceeded the duration else reverts

// == Grace Period == //
            uint128 cachedLastGracePeriodStartTimestamp = lastGracePeriodStartTimestamp;
            require(
                cachedLastGracePeriodStartTimestamp != UNSET_TIMESTAMP,
                "LiquidationLibrary: Recovery Mode grace period not started"
            );
            require(
                block.timestamp >
                    cachedLastGracePeriodStartTimestamp + recoveryModeGracePeriodDuration,
                "LiquidationLibrary: Recovery mode grace period still in effect"
            );

However, lastGracePeriodStartTimestamp can be reset by the users by sufficiently Increasing TCR past 1250000000000000000 (125%) which stops the grace period, and then decreasing it back below CCR which restarts the grace period all over again in a single transaction.

This can be done using a flashloan of stEth to call addColl() and then repay flashloan after calling withdrawColl(). Both functions calls _adjustCdpInternal() which checks new TCR in the certain modes but also in turn stops or starts the Grace Period.

// == Grace Period == //
            // We are in RM, Edge case is Depositing Coll could exit RM
            // We check with newTCR
            if (_vars.newTCR < CCR) {
                // Notify RM
                cdpManager.notifyStartGracePeriod(_vars.newTCR);
            } else {
                // Notify Back to Normal Mode
                cdpManager.notifyEndGracePeriod(_vars.newTCR);
            }

Example Attack Scenario:

• System enters recovery mode and begins Grace Period
• Grace Period is almost up and liquations are to begin attempted, attacker sees this and increases TCR to stop Grace Period using flashloans
• However, will have to pay back loan so calls withdrawColl() which decreases back the TCR and starts Grace Period with a new timestamp
• Liquidation is attempted assuming the Grace Period duration was over but reverts due to the check below since LastGracePeriodStartTimestamp has now been reset with a new timestamp

  require(
              block.timestamp >
                  cachedLastGracePeriodStartTimestamp + recoveryModeGracePeriodDuration,
              "LiquidationLibrary: Recovery mode grace period still in effect"
          );

• Attacker performs this continually, leaving some collaterals non-liquidatable during recovery mode ( Bad debt incurred )

  Tools Used

  Visual Studio Code

Recommended Mitigation Steps

I believe there is no logical reason a user would want to add collateral and withdraw collateral in a single transaction except with the aim of trying to game the protocol. So set a time gap between addColl and withdrawColl operations of a particular user using a mapping,

timeEntered[msg.sender] = block.timestamp // (or block.number)

check

require(block.timestamp > timeEntered[msg.sender] + ENTER_PERIOD, "too soon");

consider opening and closing CDP operations as well

Assessed type

Other

[bytes032]

https://gist.github.com/GalloDaSballo/a0f9766bf7bac391f49d2d167e947de0#grace-period-can-be-denied-by-repaying

[c4-pre-sort]

bytes032 marked the issue as insufficient quality report

[c4-sponsor]

GalloDaSballo (sponsor) disputed

[GalloDaSballo]

See my gist

Repaying costs less than doing this

And exiting RM is desired

[c4-judge]

jhsagd76 marked the issue as unsatisfactory: Out of scope