Algo Ssstablecoinsss

AI First Flight #2
Beginner FriendlyDeFi
EXP
View results
Submission Details
Impact: high
Likelihood: low
Invalid

Missing @nonreentrant Guards Enable Reentrancy With Callback Tokens

Missing @nonreentrant Guards Enable Reentrancy With Callback Tokens

Description

None of the state-changing external functions in dsc_engine.vy use the @nonreentrant decorator:

@external
def deposit_collateral(token_collateral_address: address, amount_collateral: uint256):
# No @nonreentrant
@external
def liquidate(collateral: address, user: address, debt_to_cover: uint256):
# No @nonreentrant
@external
def redeem_collateral(token_collateral_address: address, amount_collateral: uint256):
# No @nonreentrant

The protocol description states: "someone could fork this codebase, swap out WETH & WBTC for any basket of assets they like, and the code would work the same."

While WETH and WBTC are standard ERC-20 tokens without callbacks, if the code is forked with ERC-777 tokens or other callback-enabled tokens, reentrancy attacks become possible.

The most critical reentrancy vector is in liquidate() (lines 112-135):

def liquidate(collateral: address, user: address, debt_to_cover: uint256):
# 1. Check health factor
starting_user_health_factor = self._health_factor(user)
assert starting_user_health_factor < MIN_HEALTH_FACTOR
# 2. Calculate amounts
token_amount_from_debt_covered = self._get_token_amount_from_usd(collateral, debt_to_cover)
bonus_collateral = (token_amount_from_debt_covered * LIQUIDATION_BONUS) // LIQUIDATION_PRECISION
# 3. Transfer collateral OUT (external call - reentrancy point)
self._redeem_collateral(collateral, token_amount_from_debt_covered + bonus_collateral, user, msg.sender)
# 4. Burn DSC (debt reduction happens AFTER the external call)
self._burn_dsc(debt_to_cover, user, msg.sender)

With an ERC-777 collateral token, step 3's IERC20.transfer() triggers a tokensReceived callback on the liquidator before step 4 executes. At this point, the user's collateral has been reduced but their debt has NOT been reduced, making their health factor even worse. A malicious liquidator contract could re-enter liquidate() again, extracting additional collateral.

Risk

Likelihood: Low -- Current deployment uses WETH/WBTC which have no callbacks. However, the protocol is explicitly designed to be forked with different tokens.

Impact: High -- With callback-enabled tokens, a malicious liquidator could drain the liquidated user's collateral through repeated re-entrant liquidation calls before debt is ever reduced.

Real-World Precedent:

  • Vyper @nonreentrant Lock Bypass / Curve Finance ($73.5M, 2023): Vyper compiler bug in versions 0.2.15-0.3.0 caused reentrancy. Note: Vyper 0.4.0 is NOT affected by this bug.

  • Lendf.Me ($25M, 2020): ERC-777 callback reentrancy in supply/borrow functions.

Proof of Concept

How the attack works (with ERC-777 collateral fork):

  1. Victim has an underwater position: 10 ERC777_TOKEN collateral, 5000 DSC debt

  2. Attacker deploys a malicious liquidator contract with a tokensReceived callback

  3. Attacker calls liquidate(ERC777_TOKEN, victim, 2500) via the malicious contract

  4. _redeem_collateral transfers collateral to the attacker contract

  5. ERC-777 tokensReceived callback fires on the attacker contract

  6. Attacker re-enters liquidate(ERC777_TOKEN, victim, 2500) again

  7. Second liquidation proceeds: victim's health factor is even worse (collateral reduced, debt unchanged)

  8. Total: attacker burned 5000 DSC but extracted double the collateral + bonus

Expected outcome: Attacker drains excess collateral from the victim's position.

def test_liquidation_execution_order_vulnerable(
self, dsce, weth, eth_usd, dsc, some_user, liquidator
):
# Setup: user has underwater position
with boa.env.prank(some_user):
weth.approve(dsce.address, COLLATERAL_AMOUNT)
dsce.deposit_collateral_and_mint_dsc(weth.address, COLLATERAL_AMOUNT, AMOUNT_TO_MINT)
eth_usd.updateAnswer(18 * 10**8) # Crash price
assert dsce.health_factor(some_user) < to_wei(1, "ether")
# Between _redeem_collateral and _burn_dsc:
# Collateral: REDUCED | Debt: UNCHANGED -> reentrancy window
collateral_before = dsce.get_collateral_balance_of_user(some_user, weth)
token_amount = dsce.get_token_amount_from_usd(weth, AMOUNT_TO_MINT)
bonus = token_amount * 10 // 100
simulated_collateral = collateral_before - (token_amount + bonus)
dsc_minted, _ = dsce.get_account_information(some_user)
assert dsc_minted == AMOUNT_TO_MINT # Debt unchanged in window!
assert simulated_collateral < collateral_before # Collateral already gone!

Recommended Mitigation

Add @nonreentrant to all state-changing external functions:

@external
@nonreentrant
def deposit_collateral(token_collateral_address: address, amount_collateral: uint256):
self._deposit_collateral(token_collateral_address, amount_collateral)
@external
@nonreentrant
def liquidate(collateral: address, user: address, debt_to_cover: uint256):
# ... liquidation logic ...
@external
@nonreentrant
def redeem_collateral(token_collateral_address: address, amount_collateral: uint256):
# ... redeem logic ...

Additionally, consider reordering liquidate() to follow strict CEI (Checks-Effects-Interactions): reduce debt BEFORE transferring collateral.

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge 5 days 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!