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 phantom sale proceeds

Author Revealed upon completion

Root + Impact

Description

  • cancelListing() sets isActive = false and returns collateral, but listing.price and listing.seller remain in storage.

  • collectUsdcFromSelling() only checks !isActive and onlySeller — both satisfied after cancellation. The seller receives price - fees for a sale that never occurred.

function cancelListing(uint256 _listingId) external {
...
// @> Sets isActive = false — the ONLY check collectUsdcFromSelling uses
s_listings[_listingId].isActive = false;
usdc.safeTransfer(listing.seller, collateralForMinting[listing.tokenId]);
collateralForMinting[listing.tokenId] = 0;
// @> listing.price and listing.seller persist in storage
}
​
function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
Listing memory listing = s_listings[_listingId];
// @> !isActive is true after cancel — not just after sale
require(!listing.isActive, "Listing must be inactive to collect USDC");
// @> Pays listing.price - fees even though no buyer ever paid
...
}

Risk

Likelihood:

  • Any whitelisted user can exploit — no sale needs to occur

  • Simple three-step attack: list at max price → cancel → collect

Impact:

  • Attacker steals up to ~$4,294 per call from other users' funds

  • Combined with Finding 1 (repeated calls), entire contract is drained

Proof of Concept

ob mints first (depositing 20 USDC collateral — the victim funds). Alice mints, lists at ~$4,000, then cancels. She then calls collectUsdcFromSelling — collecting ~$3,880 in proceeds from a sale that never happened, stolen from Bob's collateral.

function test_collectAfterCancel() public {
vm.prank(owner); dealers.whitelistWallet(alice);
vm.prank(owner); dealers.whitelistWallet(bob);
​
vm.startPrank(bob);
usdc.approve(address(dealers), type(uint256).max);
dealers.mintNft(); // deposits 20 USDC (victim funds)
vm.stopPrank();
​
vm.startPrank(alice);
usdc.approve(address(dealers), type(uint256).max);
dealers.mintNft();
dealers.list(2, uint32(4000e6));
dealers.cancelListing(2); // gets collateral back, no sale occurred
​
uint256 balBefore = usdc.balanceOf(alice);
dealers.collectUsdcFromSelling(2); // steals ~3,880 USDC
assertGt(usdc.balanceOf(alice) - balBefore, 3800e6);
vm.stopPrank();
}
​

Recommended Mitigation

The boolean isActive cannot distinguish sold from cancelled listings. Replace it with a ListingStatus enum. cancelListing sets Cancelled, buy sets Sold, and collectUsdcFromSelling strictly requires Sold.

function cancelListing(uint256 _listingId) external {
Listing memory listing = s_listings[_listingId];
- if (!listing.isActive) revert ListingNotActive(_listingId);
+ if (listing.status != ListingStatus.Active) revert ListingNotActive(_listingId);
​
- s_listings[_listingId].isActive = false;
+ s_listings[_listingId].status = ListingStatus.Cancelled;
...
}
​
function buy(uint256 _listingId) external payable {
- if (!listing.isActive) revert ListingNotActive(_listingId);
+ if (listing.status != ListingStatus.Active) revert ListingNotActive(_listingId);
​
- s_listings[_listingId].isActive = false;
+ s_listings[_listingId].status = ListingStatus.Sold;
...
}
​
function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
- require(!listing.isActive, "Listing must be inactive to collect USDC");
+ require(listing.status == ListingStatus.Sold, "Listing must be 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!