panprog - `LockstakeEngine.selectVoteDelegate` uses stored borrow rate for health calculation, making it possible to avoid liquidation by switching voteDelegate if nobody calls `jug.drip`.

[sherlock-admin3]

panprog

Medium

LockstakeEngine.selectVoteDelegate uses stored borrow rate for health calculation, making it possible to avoid liquidation by switching voteDelegate if nobody calls jug.drip.

Summary

LockstakeEngine.selectVoteDelegate function requires the vault to be healthy if the new VoteDelegate is set. This is protection against abusing the Chief's flashloan protection and VoteDelegate's reserveHatch functionality: if user's vault is unhealthy, but user intentionally calls VoteDelegate.lock, liquidator can't liquidate the vault as liquidation will revert when trying to free in the same block (Chief flashloan protection). Liquidator can then call VoteDelegate.reserveHatch, which will disable VoteDelegate.lock function for 5 blocks, allowing liquidator to liquidate the vault. However, if the user is allowed to set a new VoteDelegate for unhealthy vault, then user can simply switch to a different VoteDelegate and lock there to prevent liquidations all the time. To protect against such behaviour, user's Lockstake vault is required to be healthy to select a new VoteDelegate.

The issue is that selectVoteDelegate uses stored debt rate, which can be outdated:

    if (art > 0 && voteDelegate != address(0)) {
        (, uint256 rate, uint256 spot,,) = vat.ilks(ilk); // @audit << rate here is stored, new rate should be set by calling jug.drip
        require(ink * spot >= art * rate, "LockstakeEngine/urn-unsafe");
    }

The vat core module calculates user's debt as debt units (art) * borrow rate (rate). The rate increases every second by a set amount, however the rate increase is manual and user can call jug.drip to update the rate, however it is not required to do so anywhere in the core code. Thus the user's vault can be unhealthy (with current rate), however it's still healthy using the outdated rate from the last jug.drip call. Examining even the largest live vat collaterals reveals that jug.drip is called rather infrequently - a few times per hour or even per day.

This makes it possible for the user to abuse the VoteDelegate.lock and selectVoteDelegate if liquidator tries to call reserveHatch, preventing liquidation of unhealthy vault for up to several hours or even days, until jug.drip is called.

Root Cause

LockstakeEngine.selectVoteDelegate uses stored (outdated) borrow rate: https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/lockstake/src/LockstakeEngine.sol#L267

This allows to bypass the vault health check when changing VoteDelegate to prevent unhealthy vault liquidation.

Internal pre-conditions

1. jug.drip called infrequently (not called for several hours or days)
2. User is healthy if calculated using stored borrow rate, but unhealthy if calculated with actual current borrow rate.

External pre-conditions

None

Attack Path

1. Attacker's vault has some VoteDelegate selected
2. Attacker's vault is healthy at the last stored borrow rate, but becomes unhealthy at the current rate
3. Attacker calls VoteDelegate.lock every block to prevent liquidation in the same block
4. Liquidator, being unable to liquidate user's vault (due to revert when trying to VoteDelegate.free in the liquidation), calls VoteDelegate.reserveHatch
5. Attacker calls LockstakeEngine.selectVoteDelegate with a different VoteDelegate and calls VoteDelegate.lock on this new delegate.
6. Repeat 3.

This can continue for a long time until jug.drip is called by someone. Since this is not required, this can continue for a long time (a few hours or days) until attacker's vault is in a bad debt, causing protocol losses.

Impact

1. If the attacker's vault is large enough, attacker can DOS liquidators for a long time until the vault is in a bad debt, the losses can easily be up to 1%-10% of protocol funds.
2. Even if jug.drip is called more frequently, or no bad debt happens, liquidators are forced to call VoteDelegate.reserveHatch many times without actual liquidation happening, thus wasting gas fees. When done frequently, liquidators might be disincentivized from calling reserveHatch and liquidating such user, further exposing the protocol to uncontained losses from such attacker.

PoC

Not needed

Mitigation

Call jug.drip to get the borrow rate instead of getting it from the vat.

[sunbreak1211]

Debt increment by fee accumulation via async drip is how the whole system works. If drip was still not called, then there is not increment of debt, meaning the position hasn't have to account it for its health condition. The liquidation can't either occur if that under health condition isn't met, see: https://github.com/makerdao/dss/blob/master/src/dog.sol#L181

[panprog]

Escalate

The issue is valid. If drip is not called, liquidator can still liquidate by calling drip and bark in the same transaction, but attacker can keep changing VoteDelegate and calling lock to prevent liquidator from liquidating. Since it isn't beneficial for liquidator to call drip other than at the same time with bark, drip will not be called on its own, so that liquidations will keep reverting

[sherlock-admin3]

Escalate

The issue is valid. If drip is not called, liquidator can still liquidate by calling drip and bark in the same transaction, but attacker can keep changing VoteDelegate and calling lock to prevent liquidator from liquidating. Since it isn't beneficial for liquidator to call drip other than at the same time with bark, drip will not be called on its own, so that liquidations will keep reverting

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[00xSEV]

I would like to add some points as to why the issue should remain invalid:

• If the attacker's vault is large enough, attacker can DOS liquidators for a long time until the vault is in a bad debt, the losses can easily be up to 1%-10% of protocol funds.

  • If the vault is that big, the incentive to liquidate is also enormous, because it depends on tab. See https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/lockstake/src/LockstakeClipper.sol#L263
  • Such a big incentive makes liquidators bid high priority fees or pay big tips to private relays/block builders
  • It also incentivizes other users of the protocol to call drip to avoid losing their funds

• The issue that drip may not be called for some time is already known and taken into account by governance, who sets protocol parameters. See https://docs.makerdao.com/smart-contract-modules/rates-module/jug-detailed-documentation#tragedy-of-the-commons

  • "both Keepers and MKR holders are have an incentive to regularly call drip (the former to trigger liquidation auctions, the latter to ensure that surplus accumulates to decrease MKR supply)"

  • If drip() is called very infrequently for some collateral types (due, for example, to low overall system usage or extremely stable collateral types that have essentially zero liquidation risk), then the system will fail to collect fees on Vaults opened and closed between drip() calls. As the system achieves scale, this becomes less of a concern, as both Keepers and MKR holders are have an incentive to regularly call drip (the former to trigger liquidation auctions, the latter to ensure that surplus accumulates to decrease MKR supply); however, a hypothetical asset with very low volatility yet high risk premium might still see infrequent drip calls at scale (there is not at present a real-world example of this—the most realistic possibility is base being large, elevating rates for all collateral types).

  • Even if keepers don't have an incentive to call drip, MKR holders still do

• In practice, even if we assume that all the liquidators are naive and do not use private relays or an appropriate amount of gas to be the first (which they have every incentive to do in real life), there are generalized front-runners, discussed in many previous reports, e.g., here 8.4. When they see a profitable transaction, they will front-run it, paying up to 99+% to block builders to be the first. This makes the attack very unprofitable. The attacker needs to overbid because front-runners do not pay fees if their transaction reverts => they only pay if they are before the attacker. See https://docs.flashbots.net/flashbots-protect/quick-start

  • No failed transactions: Transactions are only included in the block if they will not revert. Users do not pay fees for failed transactions.

• At best, it will delay liquidation until some liquidator or someone else (MKR holder, front-runner) calls drip for that collateral not in the same transaction as the liquidation. https://github.com/makerdao/sherlock-contest/blob/master/README.md?plain=1#L141

  • • Delaying liquidations in the order of tens of minutes is assumed valid, there is already a big delay in Maker's oracle design.

[mrhudson890]

If this issue's escalation is accepted, #99 should be valid as well due to shared root cause, mitigation, and impact.

[WangSecurity]

Indeed, there are lots of incentives to call the drip function, still, there's no obligation and there might be a period when it's not called several hours or even days.

I believe this report has sufficient proof that this attack can lead to bad debt on users losing funds, while the attack has to profit for the attacker (as I understand). Still, the cost is relatively low, paying the gas for several hours of transactions.

Hence, I believe medium severity is indeed appropriate. Planning to accept the escalation and validate this with medium severity. I agree #99 is a duplicate, are there any other duplicates @z3s @panprog?

[IllIllI000]

@WangSecurity the docs say that it's expected that multiple parties have incentive to, and will call drip: https://docs.makerdao.com/smart-contract-modules/rates-module#who-calls-drip It is assumed that drip calls will be frequent enough such inaccuracies will be minor, at least after an initial growth period. As sunbreak1211 points out above, this is how the system is expected to work, and if the bug boils down to the fact that drip() may not be called frequently enough for a liquidation to be able to be kicked off, then that's a bug in the existing clipper contract too, and is out of scope. There is no keeper code in-scope, so stating that all keepers will call drip() in the same transaction is a guess at out-of-scope code, when the docs explicitly state that multiple parties are assumed to call it for their own reasons. jug.drip() is to assess stability fees, not to re-price collateral, and it is in fact called on its own frequently enough for that purpose https://etherscan.io/txs?a=0x19c0976f590d67707e62397c87829d896dc0f1f1

[JeffCX]

Indeed, there are lots of incentives to call the drip function, still, there's no obligation and there might be a period when it's not called several hours or even days.

I believe this report has sufficient proof that this attack can lead to bad debt on users losing funds, while the attack has to profit for the attacker (as I understand). Still, the cost is relatively low, paying the gas for several hours of transactions.

Hence, I believe medium severity is indeed appropriate. Planning to accept the escalation and validate this with medium severity. I agree #99 is a duplicate, are there any other duplicates @z3s @panprog?

Could you please consider the protocol's input as well? thanks!

https://github.com/sherlock-audit/2024-06-makerdao-endgame-judging/issues/99#issuecomment-2284631410

[mrhudson890]

Both this and #99 (in additional detail) show that even if drip is called frequently enough, liquidators are griefed:

• Either by having to call reserveHatch without achieving anything, due to the the attacker switching VD.
• Or due to reverted liquidation calls.

In either case, the liquidator is grieved out of material funds.

What's more, as detailed in #99 and mentioned in this issue, liquidations mechanism as a whole is disrupted by unliquidatable urns (deliberately or not). The additional impact of shielding of other urns from liquidations by a large number of "decoys", causes bad debt to be accumulated.

[sunbreak1211]

The scenario described is that a drip call will make the position underwater. Before drip is called it cannot be liquidated, and after drip is called it can. It is not a special situation, and calling drip is often part of the liquidation process. The fact that drip may have not been called for hours is irrelevant, the liquidating parties know to call it when it would put the position underwater.

So typically such a liquidation will involve a drip and a bark. In the report the submitter mentions a griefing attempt scenario where reserveHatch is also needed. So in such case it would be: reserveHatch, drip and bark, or: drip, reserveHatch, and bark.

In any case, once drip is called, the griefer can not move to another delegate (without re-collateralizing), so liquidations can not be blocked. Even if the griefer succeeds once in moving to another delegate by front-running drip, once drip happens it can not move to another delegate (without re-collateralizing).

So since liquidations are not delayed in this situation, there is no loss of funds / potential bad debt acrrual.

Note that the contest rules have a strong assumption on keepers being adaptive, changing their logic to accustom to this special collateral type, and the option that Maker itself will run such keepers (so no point in discussing any lack of incentives from any party).

Issue 99, which was mentioned in the comments, describes another scenario, where someone keeps on re-collateralizing their position in order to grief gas from keepers. We answered that issue already - https://github.com/sherlock-audit/2024-06-makerdao-endgame-judging/issues/99#issuecomment-2284631410 (in short, loss of funds / bad debt is not relevant since the position is being saved constantly, and gas griefing is also not, since keepers are assumed smart enough not to waste gas in this scenario per the contest rules). We assume that answer was satisfactory and that's why issue 99 was not escalated.

The above reasonings should be enough for this issue. As an extra side note, any behavior that tries to grief gas from keepers can cause the positions to be grabbed by the protocol governance, and the potential griefer will lose his entire position. That alone, plus the need to lock your position behind an exit fee (and the position needs to be large due to the dust parameter), should be enough to deter any griefing trials as described. Assuming dust of $10K and a collateralization ratio of 300%, would someone risk losing $30K of MKR to perform such short-term gas-griefing attempt?

[WangSecurity]

Agree with the sponsor above, the expected way to liquidate a position is:

reserveHatch, drip and bark, or: drip, reserveHatch, and bark.

The max possible delay of the liquidation is one block before the drip call goes through. After that, the liquidation cannot be avoided. Hence, I agree this issue is low severity since I don't believe delaying the liquidation for one block will cause a significant loss of funds qualifying for medium severity.

Planning to reject the escalation and leave the issue as it is.

[panprog]

The only way to prevent the attack shown in this issue is to call drip in a separate transaction, so this is the pre-condition as listed in the issue. Things to note are:

1. In the current live code (non-lockstake) separate call to drip is not needed - any liquidator can drip and bark in the same transaction and user can't prevent it. So this issue is not present in current code (thus not out of scope)
2. Calling only reserveHatch in a separate transaction doesn't prevent this attack, as the attacker will simply switch to a different VoteDelegate
3. README only talks about calling reserveHatch:

   It is assumed that users and keepers are aware of the reserveHatch functionality for solving vote-delegate/lockstake related DoS issues. It is assumed that keepers run by 3rd-parties and Maker integrate it to their logic and that the need for using it is constantly monitored. It is also assumed that in case it is needed the Maker community/project would set up a keeper to call reserveHatch periodically (within less than a day).

There is no specific case for calling drip the same way as reserveHatch.

4. The current MakerDAO docs do talk about incentives for calling drip for different parties, but the way it is now, it doesn't imply very frequent calls specifically for liquidation purposes since it's indeed not needed. This is also visible on-chain: drip calls are very infrequent, often with days of time between the calls, while a liquidation only needs a matter of hours to cause bad debt.

So, previously: separate transaction drip call is not needed for liquidation, drip is called by different parties very infrequently (hours and days between calls).

New lockstake: if drip behaviour is unchanged, this causes the issue described in this report. No contest docs talk about having to call drip more frequently. If keepers (or Maker or some other party) specifically calls drip in a separate transaction to overcome this issue, that'll fix this issue similar to reserveHatch. This issue is based on that reverveHatch is talked about specifically in README, but the drip is not talked about the same way, and for the keepers there is still no direct incentive to call drip in a separate transaction, besides they can be griefed for gas if they call drip (since they are not compensated for calling drip, but they can't guarantee to liquidate this user since liquidation will only happen in the next block or never, is user becomes healthy).

[WangSecurity]

As I understand from that comment drip is the function that would make the position underwater and liquidatable. Hence, it's part of the liquidation process to call drip, cause otherwise the attacker wouldn't enter the liquidatable state and others wouldn't be able to liquidate the attacker.

That's why I believe the drip calls being infrequent is a valid argument here, cause it may infrequent because the liquidations are not frequent and there's no reason to call drip.

Hence, as I understand the attack can be successful only once and the attacker will be able to avoid liquidation on the first voteDelegate, but the first VD will call drip as part of the process. Thus, the attacker won't be able to avoid the second attempt to liquidate, since they have entered the liquidatable state after the first call to drip.

Hence, I believe this issue is only low severity, planning to reject the escalation and leave the issue as it is.

[WangSecurity]

Result: Invalid Unique

[sherlock-admin4]

Escalations have been resolved successfully!

Escalation status:

• panprog: rejected