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

Unauthorized Payout After Listing Cancellation

Author Revealed upon completion

Root + Impact

Description

  • The normal behavior is that sellers receive sale proceeds only after a real purchase, while canceled listings should only unlock previously locked collateral.

  • The issue is that settlement does not verify a completed sale. After canceling a listing, the seller can still call collectUsdcFromSelling and withdraw price - fees from contract liquidity funded by other users.

function cancelListing(uint256 _listingId) external {
Listing memory listing = s_listings[_listingId];
if (!listing.isActive) revert ListingNotActive(_listingId);
require(listing.seller == msg.sender, "Only seller can cancel listing");
​
s_listings[_listingId].isActive = false;
activeListingsCounter--;
​
usdc.safeTransfer(listing.seller, collateralForMinting[listing.tokenId]);
collateralForMinting[listing.tokenId] = 0;
}
​
function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
Listing memory listing = s_listings[_listingId];
require(!listing.isActive, "Listing must be inactive to collect USDC"); // @> cancellation also makes listing inactive
// @> no "sold" state check, only inactive check
​
uint256 fees = _calculateFees(listing.price);
uint256 amountToSeller = listing.price - fees;
uint256 collateralToReturn = collateralForMinting[listing.tokenId];
​
totalFeesCollected += fees;
amountToSeller += collateralToReturn;
usdc.safeTransfer(msg.sender, amountToSeller); // @> pays seller even when no buyer ever paid
}

Risk

Likelihood:

  • This occurs whenever a seller cancels a listing and then calls settlement on the same listing id.

  • This occurs as soon as the contract holds enough USDC from other users’ mints or purchases to satisfy the unauthorized payout.

Impact:

  • Attackers can drain pooled USDC without executing any sale.

  • Honest users can face insolvency, failed withdrawals, or inability to recover collateral/proceeds.

Proof of Concept

// 1) Attacker mints and lists token at a large price (e.g., 1_000e6)
// 2) Attacker cancels listing (collateral is returned, listing now inactive)
// 3) Contract has USDC from other users (mints/buys)
// 4) Attacker calls collectUsdcFromSelling(listingId)
// 5) Attacker receives (price - fees) despite zero buyer payment

Recommended Mitigation

struct Listing {
address seller;
uint32 price;
address nft;
uint256 tokenId;
bool isActive;
+ bool wasSold;
+ bool proceedsCollected;
}
​
function buy(uint256 _listingId) external payable {
- Listing memory listing = s_listings[_listingId];
+ Listing storage listing = s_listings[_listingId];
if (!listing.isActive) revert ListingNotActive(_listingId);
​
+ listing.isActive = false;
+ listing.wasSold = true;
activeListingsCounter--;
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;
}
​
function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
- Listing memory listing = s_listings[_listingId];
+ Listing storage listing = s_listings[_listingId];
require(!listing.isActive, "Listing must be inactive to collect USDC");
+ require(listing.wasSold, "Listing was not sold");
+ require(!listing.proceedsCollected, "Proceeds already collected");
​
+ listing.proceedsCollected = true;
uint256 fees = _calculateFees(listing.price);
uint256 amountToSeller = listing.price - fees;
uint256 collateralToReturn = collateralForMinting[listing.tokenId];
+ collateralForMinting[listing.tokenId] = 0;
​
totalFeesCollected += fees;
amountToSeller += collateralToReturn;
usdc.safeTransfer(msg.sender, amountToSeller);
}

Support

FAQs

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

Give us feedback!