Algo Ssstablecoinsss
AI First Flight #2
Beginner FriendlyDeFi
EXP
View results
Previous Next
Submission Details
Impact: medium
Likelihood: low
Invalid

# [M-02] `liquidate()` violates CEI pattern, transferring collateral before burning debt, enabling reentrancy with callback tokens

webrainsec

Description

In liquidate(), _redeem_collateral (which makes an external transfer call) executes before _burn_dsc. Between these two operations, the user's collateral is reduced but their debt is unchanged, creating an intermediate state with a worse health factor that a callback-enabled token can exploit.

Vulnerability Details

# dsc_engine.vy:124-130
self._redeem_collateral( # @> Step 1: external transfer (extcall at line 253)
collateral,
token_amount_from_debt_covered + bonus_collateral,
user,
msg.sender,
)
self._burn_dsc(debt_to_cover, user, msg.sender) # @> Step 2: burn AFTER transfer

_redeem_collateral calls extcall IERC20(token_collateral_address).transfer(_to, amount_collateral) at line 253. If the collateral token has a transfer callback (ERC777 tokensReceived, ERC1363 onTransferReceived), control passes to the liquidator during step 1.

At that point, the user's collateral is reduced but debt is unchanged, so their HF is even worse. The liquidator can re-enter liquidate() against the same user, triggering cascading liquidations that extract more collateral than intended.

No function in dsc_engine.vy uses @nonreentrant.

The companion function redeem_collateral_for_dsc (line 94-103) correctly orders burn before redeem, confirming this is an oversight rather than a design choice.

Risk

Likelihood:

  • The README says "someone could fork this codebase, swap out WETH & WBTC for any basket of assets they like." Forks using ERC777/ERC1363 tokens are explicitly in scope.

  • No @nonreentrant guard exists on any function.

Impact:

  • A malicious liquidator can drain a user's entire collateral through cascading reentrant liquidations.

  • With standard WETH/WBTC this is unexploitable, but the codebase is designed for forking with arbitrary tokens.

Proof of Concept

Attack sequence with ERC777 collateral:

  1. User has 10 tokens collateral ($20,000), 10,000 DSC debt, HF = 0.95e18 (liquidatable)

  2. Attacker calls liquidate(token, user, 1000e18) covering $1,000 debt

  3. _redeem_collateral transfers 1,100 worth of collateral to attacker via extcall IERC20.transfer

  4. ERC777 tokensReceived fires on attacker's contract

  5. Attacker re-enters liquidate(token, user, 1000e18) — user now has less collateral, same debt, even worse HF

  6. Step 5 repeats until user's collateral is drained

  7. Outer calls return, _burn_dsc executes for each, but collateral is already gone

Recommendations

Add @nonreentrant and reorder to burn debt before redeeming collateral:

@external
+ @nonreentrant
def liquidate(collateral: address, user: address, debt_to_cover: uint256):
assert debt_to_cover > 0, "DSCEngine__NeedsMoreThanZero"
starting_user_health_factor: uint256 = self._health_factor(user)
assert (
starting_user_health_factor < MIN_HEALTH_FACTOR
), "DSCEngine__HealthFactorOk"
token_amount_from_debt_covered: uint256 = self._get_token_amount_from_usd(
collateral, debt_to_cover
)
bonus_collateral: uint256 = (
token_amount_from_debt_covered * LIQUIDATION_BONUS
) // LIQUIDATION_PRECISION
+ self._burn_dsc(debt_to_cover, user, msg.sender)
self._redeem_collateral(
collateral,
token_amount_from_debt_covered + bonus_collateral,
user,
msg.sender,
)
- self._burn_dsc(debt_to_cover, user, msg.sender)
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 3 hours ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!