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

NFTDealers::cancelListing incorrectly returns collateral allowing sellers to get free NFTs

Author Revealed upon completion

NFTDealers::cancelListing incorrectly returns collateral allowing sellers to get free NFTs

Description

  • When a seller mints an NFT, lockAmount USDC is locked as collateral in the contract. This collateral is intended to be returned only when the NFT is sold via collectUsdcFromSelling. Cancelling a listing should only delist the NFT while keeping the collateral locked.

  • cancelListing returns the full collateral to the seller and then wipes collateralForMinting[tokenId] = 0. This creates two compounding issues: the seller receives their collateral back while still holding the NFT — effectively minting for free

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--;
​
//@> collateral returned to seller on cancel
//@> seller keeps the NFT AND gets collateral back
usdc.safeTransfer(listing.seller, collateralForMinting[listing.tokenId]);
//@> mapping permanently wiped — can never be recovered
collateralForMinting[listing.tokenId] = 0;
​
emit NFT_Dealers_ListingCanceled(_listingId);
}

Risk

Likelihood: High

  • Any whitelisted seller who lists and then cancels their listing triggers this automatically — no special setup required

Impact: High

  • Seller receives collateral back AND keeps the NFT

Proof of Concept

function test_CancelListingCollateralBug() public revealed whitelisted() {
uint256 tokenId = 1;
uint256 nftPrice = 1000e6;
​
mintAndListNFTForTesting(tokenId, nftPrice);
​
console.log("collateral before cancel:", nftDealers.collateralForMinting(tokenId));
console.log("userWithCash USDC before:", usdc.balanceOf(userWithCash));
​
// cancel listing
vm.prank(userWithCash);
nftDealers.cancelListing(tokenId);
​
console.log("collateral after cancel :", nftDealers.collateralForMinting(tokenId));
console.log("userWithCash USDC after :", usdc.balanceOf(userWithCash));
console.log("userWithCash owns NFT :", nftDealers.ownerOf(tokenId) == userWithCash);
​
// seller has collateral back AND still owns NFT — free mint
assertEq(usdc.balanceOf(userWithCash), nftDealers.lockAmount());
assertEq(nftDealers.ownerOf(tokenId), userWithCash);
assertEq(nftDealers.collateralForMinting(tokenId), 0);
}

Output

Logs:
collateral before cancel: 20000000
userWithCash USDC before: 0
collateral after cancel : 0
userWithCash USDC after : 20000000
userWithCash owns NFT : true

Recommended Mitigation

cancelListing should only delist the NFT — collateral must remain
locked until the NFT is sold or burned.

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;
​
emit NFT_Dealers_ListingCanceled(_listingId);
}

If collateral return on cancel is intended, a separate burnNft
function should be introduced that burns the NFT and returns collateral
atomically — ensuring the seller can never hold both the NFT and the
collateral simultaneously.

Support

FAQs

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

Give us feedback!