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

[H-02] cancelListing incorrectly refunds minting collateral without burning the NFT, enabling free mints

Author Revealed upon completion

Description

Normal behavior dictates that users must lock a 20 USDC collateral within the protocol to mint an NFT.

The specific issue is that the cancelListing function refunds this collateral to the seller but does not require the NFT to be burned or transferred back to the protocol, allowing users to keep both the NFT and their initial USDC.

Solidity
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 refunded, but the NFT remains in the seller's wallet
usdc.safeTransfer(listing.seller, collateralForMinting[listing.tokenId]);
collateralForMinting[listing.tokenId] = 0;

emit NFT_Dealers_ListingCanceled(_listingId);
}

Risk

Likelihood:

Whitelisted users who mint an NFT can immediately list it and call cancelListing in the same or subsequent transaction.

No complex preconditions or specific states are required to execute this action.

Impact:

The protocol suffers a complete loss of its intended economic design, as the 20 USDC collateral per NFT is drained.

A malicious actor can loop this process to mint the entire MAX_SUPPLY of the collection for absolutely zero net cost, stealing the protocol's backing collateral.

Proof of Concept

Solidity
function test_PoC_FreeNFTAndCollateralTheft() public {
uint256 initialUsdc = usdc.balanceOf(attacker);

vm.startPrank(attacker);
// 1. Attacker pays 20 USDC and mints the NFT
dealers.mintNft();
assertEq(usdc.balanceOf(attacker), initialUsdc - 20e6);
​
// 2. Attacker lists the NFT
dealers.list(1, 100e6);
​
// 3. Attacker cancels the listing and gets the collateral back
dealers.cancelListing(1);
vm.stopPrank();
​
// Asserts the attacker got the money back but kept the NFT
assertEq(usdc.balanceOf(attacker), initialUsdc);
assertEq(dealers.ownerOf(1), attacker);
}

Recommended Mitigation

Canceling a secondary market listing should only change the listing status to inactive. It should not refund the minting collateral, since the user still retains ownership of the NFT.

Diff
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!