Cowrie Research

omni-protocol-doodles-nftx-safe-transfer-double-reentrancy-2022-07-10

11 min read

Omni Protocol Doodles / NFTX safeTransferFrom Double Reentrancy (2022-07-10)

  • Name: Omni Protocol Doodles / NFTX safeTransferFrom double reentrancy
  • Date: 2022-07-10
  • Protocol: Omni Protocol
  • Chain: Ethereum
  • Category: NFT lending-market integration failure / ERC-721 safe-transfer callback reentrancy / collateral-flag desynchronization
  • Loss: more than 1,300 ETH (about $1.4M at the time)
  • Summary: Omni’s NFT money market treated ERC-721 collateral transfers like passive asset movement. They were not passive. In Omni Pool 0x50c7a557d408a5f5a7fdbe1091831728ae7eba45, both executeWithdrawERC721() and executeERC721LiquidationCall() called NToken.burn() before finalizing userConfig.setUsingAsCollateral(...). That burn path eventually reached ERC-721 safeTransferFrom(), which handed execution to attacker-controlled onERC721Received() logic. The attacker used Doodles NFTs sourced through NFTX, first to re-enter during collateral withdrawal, then again during liquidation, so Omni first let WETH be borrowed against NFT collateral and then let the market forget that the debt still had collateral constraints attached.
  • Trigger / entry point:
    • Omni’s withdraw and liquidation flows burned receipt-side NFT positions before finalizing collateral-usage state.
    • NToken.burn() transferred ERC-721 collateral out through safeTransferFrom(), which invokes onERC721Received() on contract recipients.
    • The attacker made the withdrawal destination and liquidation recipient attacker-controlled contracts, so each NFT transfer became a callback opportunity.
  • Root cause:
    • Omni violated checks-effects-interactions in two linked places: SupplyLogic.executeWithdrawERC721() and LiquidationLogic.executeERC721LiquidationCall().
    • In both flows, NToken.burn() reached ERC-721 safeTransferFrom() before the pool finalized userConfig.setUsingAsCollateral(...), so the receiver hook ran while Omni still believed the old collateral relationship was live.
    • The liquidation side was especially dangerous because the relevant setUsingAsCollateral(..., false) cleanup sat on the vars.userGlobalData.balance == 1 path, so the attacker could re-enter exactly as the market was about to retire the last collateral bit for that reserve.
    • That ordering let the attacker re-enter while debt, liquidation eligibility, and collateral-usage flags still described an earlier state, then resume into stale cleanup writes that erased the renewed collateral relationship.
  • Exploit path:
    1. The attacker flash-loaned WETH from Balancer and NFTX Doodles vault tokens, then redeemed enough Doodles NFTs to stage the attack.
    2. An attacker-controlled DebtTaker contract supplied three Doodles NFTs as collateral to Omni and borrowed WETH against them.
    3. DebtTaker then called withdrawERC721() to remove two of those NFTs to an attacker-controlled Liquidator contract.
    4. During the second NFT’s safeTransferFrom() callback, Liquidator.onERC721Received() re-entered Omni and liquidated the now-underwater final NFT collateral.
    5. The liquidation path itself also transferred the NFT through safeTransferFrom(), creating a second callback window before Omni cleared collateral-usage state.
    6. Inside that liquidation callback, the attacker moved the recovered NFTs back to DebtTaker, resupplied them as collateral, and borrowed as much WETH as possible a second time.
    7. When the liquidation and withdrawal frames resumed, Omni finished by setting userConfig.setUsingAsCollateral(...) to false, effectively forgetting that the renewed debt still depended on live collateral.
    8. With the debt checks now bypassed, DebtTaker withdrew the remaining NFTs and the attacker repaid the flash loans while keeping the excess WETH.
  • What the attacker needed to be true:
    • Omni had to allow NFT collateral withdrawal and liquidation paths to reach attacker-controlled ERC-721 receiver callbacks before sealing collateral bookkeeping.
    • The attacker needed enough NFT collateral and Omni WETH liquidity for the double-reentrancy path to be worth staging.
    • The market had to rely on usingAsCollateral-style flags during later debt checks so clearing that flag at the wrong moment would suppress solvency enforcement.
  • Technical anatomy worth remembering:
    • This was not an ERC-777 or arbitrary-token-hook incident. The callback surface came from ordinary ERC-721 safeTransferFrom() semantics inside an NFT lending market.
    • The attack was doubly stateful: the first reentry point exploited Omni’s withdrawal path, and the second exploited its liquidation path, with both bugs sharing the same burn before collateral-flag update mistake.
    • The attacker did not need a magical callback target; they only needed Omni to honor contract recipients on collateral release. DebtTaker.withdrawERC721() pointed the withdrawn NFTs at the attacker-controlled Liquidator, and the second NFT transfer was enough to catch the account in the 1 NFT left, 3-NFT-sized debt still open state required for liquidation.
    • The decisive profit step came after the liquidation callback: Omni resumed and cleared usingAsCollateral as though the position had been unwound, even though the attacker had already moved the NFTs back, resupplied them, and borrowed again during the callback window.
    • Immunefi’s reconstruction preserves the exact vulnerable modules: SupplyLogic.executeWithdrawERC721() and LiquidationLogic.executeERC721LiquidationCall() in the Omni Pool.
    • The exploit is a strong reminder that safeTransferFrom() means “call untrusted receiver code if the destination is a contract,” not “perform a safer transfer.”
  • Representative transactions and addresses:
    • Main exploit transaction: 0x05d65e0adddc5d9ccfe6cd65be4a7899ebcb6e5ec7a39787971bcc3d6ba73996
    • Vulnerable Omni Pool contract: 0x50c7a557d408a5f5a7fdbe1091831728ae7eba45
    • Vulnerable withdraw path reference: SupplyLogic.executeWithdrawERC721() on Omni Pool code (#F7, around line 268 on Etherscan’s verified source view)
    • Vulnerable liquidation path references: LiquidationLogic.executeERC721LiquidationCall() on Omni Pool code (#F9, around lines 436 and 465 on Etherscan’s verified source view)
  • Response / mitigation:
    • Omni paused the protocol after the exploit.
    • The practical fix direction was straightforward even if the deployed loss was not: retire usingAsCollateral and other debt-gating state before any outbound ERC-721 safeTransferFrom() can hand control to the recipient, or stage the release so callback-reachable paths cannot rely on stale collateral truth.
    • The exploit became a widely cited NFT-lending example of why ERC-721 receiver callbacks need the same suspicion as ERC-777 hooks or raw ETH sends.
    • Immunefi later published a full local-fork proof of concept using fork block 15114361, which is a useful review artifact because it preserves the exact two-contract choreography (DebtTaker plus Liquidator) auditors should replay when testing withdraw -> receiver hook -> liquidation -> receiver hook -> resupply -> reborrow -> stale cleanup chains.

Pattern tags

Why the exploit worked

  • Omni authenticated the wrong object boundary. Both withdrawal and liquidation treated NToken.burn() as if it merely moved an NFT, but economically that burn was also the point where collateral membership, liquidation eligibility, and borrow authority were supposed to change.
  • Because safeTransferFrom() handed control to attacker contracts before userConfig.setUsingAsCollateral(..., false) executed, receiver hooks could act against a market state that still said this debt is backed even while the backing NFT was already leaving.
  • The second callback mattered because it let the attacker do more than just escape collateral once. During liquidation, Omni again transferred the NFT before finalizing the last-collateral cleanup, so the attacker could move the NFT back, resupply it, and borrow again before the outer frame wrote the stale not collateral answer.
  • Profit came from stale cleanup overwriting renewed reality. When the original withdrawal and liquidation frames resumed, Omni retired collateral membership based on the pre-callback picture, silently disconnecting live debt from live NFT backing.
  • That makes Omni a strong reminder that withdrawal paths and liquidation paths are one shared solvency state machine. If either path can observe or publish usingAsCollateral at the wrong time, a composed attacker flow can turn two half-bugs into one full reserve drain.

Detection and audit pivots

  • Review every ERC-721 collateral withdraw, seize, redeem, and liquidation path as an external-call surface even when the token itself is honest. safeTransferFrom() to a contract recipient is a callback boundary inside solvency code.
  • Map exactly when collateral-membership bits, debt-dependence state, and liquidation eligibility are retired relative to outbound NFT delivery. If the answer is after transfer, assume a receiver hook can test the old state story.
  • Compare sibling paths together, not one at a time. Omni paid because both withdraw and liquidation used the same burn first, retire collateral later ordering and the attacker chained the two windows.
  • Stress the last collateral item branch. Cleanup conditioned on balance == 1, remainingSupply == 0, or similar near-empty-state checks is often where stale membership retirement becomes reachable.
  • Add adversarial tests where the NFT receiver re-enters into liquidation, resupply, reborrow, or another collateral-management helper before the original frame finishes. The profitable sequence here was withdraw -> callback -> liquidation -> callback -> resupply -> reborrow -> stale cleanup.
  • Audit whether resumed outer frames can overwrite a newer collateral truth established during the callback window. Any setUsingAsCollateral(false) or clear membership write needs to prove the underlying position still matches the frame that scheduled it.

Why this incident matters

  • It fills an important reentrancy subfamily that does not rely on ERC-777 hooks, malicious token admission, or raw ETH transfer callbacks.
  • It shows how NFT lending markets inherit callback risk directly from ERC-721 receiver semantics.
  • It also preserves a useful control lesson: a protocol can mis-handle both withdrawal and liquidation in the same market, and the profitable exploit may require composing those two half-bugs into one state-desynchronization chain.
  • The preserved healthy boundary was simple: collateral-membership retirement needed to be final before receiver-hook delivery, not cleaned up afterward. That makes Omni a strong handoff into the broader configure first, deliver second design layer rather than only a generic reentrancy happened story.

Failure-prevention checklist

  • Retire collateral-membership and debt-dependence state before any ERC-721 safeTransferFrom() can deliver the NFT to a receiver hook.
  • If business logic requires staged withdrawal or liquidation, split it into finalize accounting first, claim asset second rather than letting one frame both decide solvency and perform callback-bearing delivery.
  • Make resumed cleanup idempotent against newer state. A delayed setUsingAsCollateral(false) should fail closed if the NFT was re-supplied or the position changed during nested execution.
  • Apply the same callback-ordering invariant to withdrawal, liquidation, rescue, refinance, and migration paths. NFT lending systems often patch the headline withdraw flow and leave a secondary seize path behind.
  • Treat last NFT / last collateral unit branches as high-risk state transitions and fuzz them with receiver-hook reentry.
  • Prefer frame-local entitlement objects over ambient account flags when possible. If a liquidation frame cannot prove it still owns the right to clear collateral state, it should not be allowed to publish that write.
  • Keep PoC tests for composed receiver-hook flows in CI, including withdraw -> liquidate -> resupply -> reborrow choreography across multiple attacker-controlled contracts.

Audit prompts

  • Which exact lines retire usingAsCollateral, debt dependence, or liquidation eligibility relative to outbound ERC-721 delivery?
  • Can any withdrawal, liquidation, or seize path transfer NFTs to a contract recipient before the market has committed the new collateral state?
  • If a callback re-supplies the NFT or opens new debt, what prevents the resumed outer frame from writing an older not collateral answer over the new reality?
  • Are there separate code paths for ordinary withdrawal and liquidation, and do they enforce the same callback and membership-ordering invariants?
  • Do last collateral item branches rely on stale balances or counts captured before the callback rather than recomputing against post-callback state?
  • Can a receiver hook reach borrow, resupply, liquidation, or collateral-toggle functions that assume the original frame has already finished?
  • Does the protocol ever rely on EOA-like recipient assumptions for NFT delivery, or does it explicitly model hostile contract receivers?
  • What test proves that no sequence of receiver hooks can cause debt to survive after the market thinks all supporting NFT collateral has been retired?

Lessons that should already be standard

  • ERC-721 safeTransferFrom() is an external call, not a harmless transfer primitive. NFT lending code has the same callback problem space as ERC-777 or raw ETH refunds.
  • Withdrawal and liquidation are one solvency object. If both paths can publish collateral membership, they must share one ordering discipline.
  • Last item cleanup code is often the real attack surface. Near-empty-state branches deserve more suspicion, not less.
  • Stale cleanup can be as dangerous as missing cleanup. Many exploits pay because an old frame keeps authority to publish state after nested execution has already changed reality.
  • Configure first, deliver second applies just as much to collateral release as to token minting or vault withdrawals.
  • Receiver-hook PoCs should be normal audit artifacts for NFT lending markets, not exotic edge-case tests.

Internal linkages

Control surface

Graph View

Backlinks