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

M06. Re-listing an NFT after purchase permanently locks the original seller's proceeds and redirects their collateral to the new seller

Author Revealed upon completion

Root + Impact

Description

  • After a sale through buy(), the original seller is expected to call collectUsdcFromSelling() to receive their sale proceeds and recover their locked collateral. There is no deadline or urgency for this call — a seller may legitimately delay collecting.

  • list() allows a new owner to relist a token as soon as the previous listing is inactive. When they do, the entire s_listings[tokenId] entry is overwritten with the new seller's address and price. Because s_listings is keyed by tokenId (not by a unique listing ID), there is no separate slot for the original seller's pending claim.

  • Once the mapping is overwritten, the onlySeller modifier permanently blocks the original seller from calling collectUsdcFromSelling() or updatePrice(). Because collateralForMinting[tokenId] was never zeroed after the original sale, it is now accessible to whichever party next calls collectUsdcFromSelling() or cancelListing() — i.e., the new seller.

// s_listings is keyed by tokenId, not by a unique listing counter
mapping(uint256 => Listing) public s_listings;
​
function list(uint256 _tokenId, uint32 _price) external onlyWhitelisted {
// ...
require(s_listings[_tokenId].isActive == false, "NFT is already listed");
// @> overwrites the previous seller's address and price — no history preserved
s_listings[_tokenId] =
Listing({seller: msg.sender, price: _price, nft: address(this), tokenId: _tokenId, isActive: true});
emit NFT_Dealers_Listed(msg.sender, listingsCounter);
}
​
modifier onlySeller(uint256 _listingId) {
// @> after re-listing, this check now points to the NEW seller — original seller is permanently locked out
require(s_listings[_listingId].seller == msg.sender, "Only seller can call this function");
_;
}
​
function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
// ...
// @> collateralForMinting[tokenId] was never zeroed after the original sale
uint256 collateralToReturn = collateralForMinting[listing.tokenId];
// @> the new seller receives the original minter's locked collateral
usdc.safeTransfer(msg.sender, amountToSeller);
}

Risk

Likelihood: Medium

  • The trigger requires two conditions to align: (1) the new owner must be whitelisted (list() enforces onlyWhitelisted), and (2) they must relist before the original seller calls collectUsdcFromSelling(). Whitelisting is owner-controlled, so the re-lister is always a vetted participant.

  • In normal marketplace use, NFT flipping (buy then immediately relist) is a common pattern. Any whitelisted flipper will trigger this silently without realising the original seller has not yet collected — no malicious intent required.

Impact: High

  • The original seller permanently loses their entire sale proceeds (listing.price - fees) and their locked collateral (lockAmount). There is no recovery path — no admin function can reassign the frozen funds.

  • The new seller who eventually calls collectUsdcFromSelling() or cancelListing() receives the original minter's lockAmount as a windfall, draining funds they are not entitled to.

Proof of Concept

Setup: seller mints tokenId=1 and has 20 USDC locked as collateral.

  1. Seller lists tokenId=1 at 100 USDC. s_listings[1].seller = seller.

  2. Buyer purchases tokenId=1. Buyer pays 100 USDC. s_listings[1].isActive = false. collateralForMinting[1] = 20e6 (never zeroed by buy()).

  3. Seller has not yet collected — this is normal; there is no deadline.

  4. Buyer immediately relists tokenId=1 at 50 USDC. list(1, 50e6) executes without error and overwrites s_listings[1].seller = buyer.

  5. Seller calls collectUsdcFromSelling(1). The onlySeller modifier checks s_listings[1].seller == seller but the stored seller is now buyer — the call reverts.

  6. Buyer sells to a third party and calls collectUsdcFromSelling(1). Buyer receives 50 - fee + 20 (seller's collateral) = ~67 USDC instead of the ~47 USDC they are entitled to, pocketing the original seller's 20 USDC collateral.

function test_relist_locksOriginalSellerProceeds() public {
uint32 nftPrice = 100e6;
​
// Seller lists tokenId=1
vm.prank(seller);
nftDealers.list(1, nftPrice);
​
// Buyer purchases — seller's proceeds are now pending collection
vm.startPrank(buyer);
usdc.approve(address(nftDealers), nftPrice);
nftDealers.buy(1);
vm.stopPrank();
​
// collateralForMinting[1] is still 20 USDC — not zeroed after buy()
assertEq(nftDealers.collateralForMinting(1), LOCK_AMOUNT);
​
// Buyer immediately relists — overwrites s_listings[1].seller to buyer
vm.prank(owner);
nftDealers.whitelistWallet(buyer); // buyer needs whitelist to list
vm.startPrank(buyer);
nftDealers.list(1, 50e6);
vm.stopPrank();
​
// Original seller can no longer collect — permanently locked out
vm.prank(seller);
vm.expectRevert("Only seller can call this function");
nftDealers.collectUsdcFromSelling(1);
​
// Buyer sells to a third party, then collects the original seller's collateral as a windfall
address thirdParty = makeAddr("thirdParty");
usdc.mint(thirdParty, 50e6);
vm.startPrank(thirdParty);
usdc.approve(address(nftDealers), 50e6);
nftDealers.buy(1);
vm.stopPrank();
​
uint256 buyerBefore = usdc.balanceOf(buyer);
vm.prank(buyer);
nftDealers.collectUsdcFromSelling(1);
uint256 buyerReceived = usdc.balanceOf(buyer) - buyerBefore;
​
// Buyer received sale proceeds (50 - fee) PLUS the original seller's 20 USDC collateral
uint256 fees = nftDealers.calculateFees(50e6);
assertEq(buyerReceived, 50e6 - fees + LOCK_AMOUNT, "buyer received original seller's collateral");
​
// Seller's 100 USDC proceeds and 20 USDC collateral are permanently lost
// The contract has no mechanism to return them
}

Recommended Mitigation

The root cause is that s_listings uses tokenId as its key, so re-listing destroys the previous seller's claim. The fix requires decoupling the listing record from the token ID by keying it on the monotonic listingsCounter and preventing re-listing until the previous seller has collected.

In list(), store under listingsCounter and block re-listing while proceeds are uncollected:

+ mapping(uint256 => bool) private s_collected;
​
function list(uint256 _tokenId, uint32 _price) external onlyWhitelisted {
require(_price >= MIN_PRICE, "Price must be at least 1 USDC");
require(ownerOf(_tokenId) == msg.sender, "Not owner of NFT");
require(s_listings[_tokenId].isActive == false, "NFT is already listed");
+ // Block re-listing if prior sale proceeds have not been collected
+ require(collateralForMinting[_tokenId] == 0 || s_collected[_tokenId], "Prior proceeds uncollected");
​
listingsCounter++;
activeListingsCounter++;
​
- s_listings[_tokenId] = Listing({...});
+ s_listings[listingsCounter] = Listing({seller: msg.sender, price: _price, nft: address(this), tokenId: _tokenId, isActive: true});
emit NFT_Dealers_Listed(msg.sender, listingsCounter);
}

Note: fully fixing this requires also resolving H-02 (keying all operations on listingsCounter rather than tokenId).

Support

FAQs

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

Give us feedback!