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

Canceled listings can still be settled as sold

Author Revealed upon completion

Root + Impact

NFTDealers::collectUsdcFromSelling() only checks whether a listing is inactive and does not verify that a sale actually occurred,
a seller can withdraw sale proceeds from a canceled listing as if the NFT had been sold.

Description

  • The intended behavior is that collectUsdcFromSelling() should only be callable after a successful purchase, when then protocol has actually received the buyer’s USDC and the listing has transitioned into a sold state. A canceled listing should not create any claimable sale proceeds because no sale took place.

  • The issue is that collectUsdcFromSelling() treats every inactive listing as eligible for settlement, without distinguishing between a listing that was sold through buy() and a listing that was merely deactivated through cancelListing(). As a result, after canceling a listing, the seller can still call collectUsdcFromSelling() and receive listing.price - fees as if the contract had collected payment from a buyer.

​
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);
}

Risk

Likelihood: High

  • The issue occurs through normal protocol usage after a seller cancels a listing.

  • No special privileges are required beyond owning and listing an NFT.

Impact: High

  • A seller can receive sale proceeds even though no buyer ever paid for the NFT.

  • This allows direct USDC extraction from the contract and also corrupts fee accounting by increasing totalFeesCollected on a nonexistent sale.

Proof of Concept

The following test shows that after canceling a listing, the seller can still call collectUsdcFromSelling() and receive a payout despite no purchase having occurred.

function test_CanceledListing_CanStillBeSettledAsSold() public revealed whitelisted {
uint256 salePrice = 2_000e6; // 2,000 USDC
uint256 fees = nftDealers.calculateFees(salePrice);
uint256 expectedFakePayout = salePrice - fees;
​
// user1 mints and lists tokenId 1.
deal(address(usdc), user1, LOCK_AMOUNT);
vm.startPrank(user1);
usdc.approve(address(nftDealers), LOCK_AMOUNT);
nftDealers.mintNft();
nftDealers.list(1, uint32(salePrice));
​
// user1 cancels the listing, so no buyer has ever paid salePrice.
nftDealers.cancelListing(1);
​
// Seed the contract so the invalid settlement can be observed.
deal(address(usdc), address(nftDealers), 10_000e6);
​
uint256 userBalanceBefore = usdc.balanceOf(user1);
​
// The seller can still collect as if the listing had been sold.
nftDealers.collectUsdcFromSelling(1);
​
assertEq(usdc.balanceOf(user1) - userBalanceBefore, expectedFakePayout);
vm.stopPrank();
}

Recommended Mitigation

Track whether a listing was actually sold, and allow collectUsdcFromSelling() only for listings that were completed through buy(). A canceled listing should never become claimable for sale proceeds. While applying this fix, mark the listing as sold and inactive before external interactions in buy() so the function also follows the checks-effects-interactions pattern more closely.

+ mapping(uint256 => bool) public listingWasSold;
​
function buy(uint256 _listingId) external payable {
(... )
​
activeListingsCounter--;
​
+ listingWasSold[_listingId] = true;
+ s_listings[_listingId].isActive = false;
​
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;
​
emit NFT_Dealers_Sold(msg.sender, listing.price);
}
​
function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
Listing memory listing = s_listings[_listingId];
require(!listing.isActive, "Listing must be inactive to collect USDC");
+ require(listingWasSold[_listingId], "Listing was not sold");
​
uint256 fees = _calculateFees(listing.price);
​
(... )
}

Support

FAQs

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

Give us feedback!