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

Collateral Leak via `cancelListing`

Author Revealed upon completion

cancelListing returns collateral leads to free mint for whitelisted user

Description

  • Minting collateral (20 USDC) should remain locked as long as the NFT exists, or until it is sold (at which point it is returned to the original minter).Explain the specific issue or problem in one or more sentences. However, cancelListing returns the collateral to the seller immediately, but does not require the NFT to be burned or returned to the contract.

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 is returned to seller here
usdc.safeTransfer(listing.seller, collateralForMinting[listing.tokenId]);
​
collateralForMinting[listing.tokenId] = 0;
​
emit NFT_Dealers_ListingCanceled(_listingId);
}

Risk

Likelihood:

  • Any whitelisted user who mints an NFT can immediately list() and cancelListing() to reclaim their 20 USDC while keeping the NFT.

Impact:

  • This allows "Free Minting" of the protocol's NFTs, bypassing the intended economic lock.

  • Impact 2

Proof of Concept

function testCollateralLeakViaCancel_Bug() public revealed {
uint256 tokenId = 1;
uint256 nftPrice = 1000e6;
​
// 1. User mints NFT (Locks 20 USDC)
mintNFTForTesting();
assertEq(usdc.balanceOf(userWithCash), 0);
assertEq(nftDealers.ownerOf(tokenId), userWithCash);
​
// 2. User lists NFT
vm.prank(userWithCash);
nftDealers.list(tokenId, uint32(nftPrice));
​
// 3. User cancels listing
vm.prank(userWithCash);
nftDealers.cancelListing(tokenId);
​
// 4. Demonstration of Leak: User has their 20 USDC back AND still has the NFT
assertEq(usdc.balanceOf(userWithCash), 20e6, "User should have reclaimed collateral");
assertEq(nftDealers.ownerOf(tokenId), userWithCash, "User should still own the NFT");
assertEq(usdc.balanceOf(address(nftDealers)), 0, "Contract should be empty");
}

Recommended Mitigation

Remove the collateral return from cancelListing. Collateral should only be returned when the NFT is sold (via collectUsdcFromSelling)

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

Support

FAQs

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

Give us feedback!