NFT Dealers
First Flight #58
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Collateral Leak via `cancelListing`

howiecht

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

Lead Judging Commences

rubik0n Lead Judge 16 days ago
Submission Judgement Published
Validated
Assigned finding tags:

free-mint-when-cancel

No flag when the listing is canceled.

Support

FAQs

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

Give us feedback!