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

Listing overwrite permanently locks previous seller's uncollected sale proceeds

Author Revealed upon completion

Root + Impact

Description

  • After buy() completes, the seller calls collectUsdcFromSelling(tokenId) to receive proceeds. The listing data persists at s_listings[tokenId] with isActive = false.

  • list() stores new listings at the same s_listings[_tokenId] slot. When the buyer relists the NFT, the original seller's address is overwritten. Their collectUsdcFromSelling reverts on onlySeller because the seller field now points to the new owner.

s_listings[_tokenId] =
Listing({seller: msg.sender, price: _price, nft: address(this), tokenId: _tokenId, isActive: true});
// @> overwrites previous seller's data at same storage key

Risk

Likelihood:

  • Any buyer who relists before the previous seller collects triggers this — normal marketplace activity, no malice required

Impact:

  • Seller permanently loses sale proceeds plus minting collateral with no recovery path

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.34;
​
import {Test} from "forge-std/Test.sol";
import {NFTDealers} from "../../src/NFTDealers.sol";
import {MockUSDC} from "../../src/MockUSDC.sol";
​
contract Exploit_ListingOverwrite is Test {
NFTDealers nftDealers;
MockUSDC usdc;
address owner = makeAddr("owner");
address alice = makeAddr("alice");
address bob = makeAddr("bob");
​
function setUp() public {
usdc = new MockUSDC();
nftDealers = new NFTDealers(owner, address(usdc), "NFTD", "NFTD", "img", 20e6);
usdc.mint(alice, 100e6);
usdc.mint(bob, 200e6);
vm.startPrank(owner);
nftDealers.revealCollection();
nftDealers.whitelistWallet(alice);
nftDealers.whitelistWallet(bob);
vm.stopPrank();
vm.startPrank(alice);
usdc.approve(address(nftDealers), 20e6);
nftDealers.mintNft();
vm.stopPrank();
}
​
function testExploit_ListingOverwrite() public {
vm.startPrank(alice);
nftDealers.list(1, 100e6);
vm.stopPrank();
​
vm.startPrank(bob);
usdc.approve(address(nftDealers), 100e6);
nftDealers.buy(1);
nftDealers.list(1, 50e6); // overwrites s_listings[1].seller = bob
vm.stopPrank();
​
vm.startPrank(alice);
vm.expectRevert("Only seller can call this function");
nftDealers.collectUsdcFromSelling(1);
vm.stopPrank();
}
}

Run with forge test --match-test testExploit_ListingOverwrite. Alice's collectUsdcFromSelling reverts because s_listings[1].seller now points to Bob. Alice's 119 USDC (99 proceeds + 20 collateral) is permanently locked.

Recommended Mitigation

Use listingsCounter as the storage key instead of tokenId, so each sale gets its own slot that is never overwritten by future listings of the same NFT. This ensures the original seller's listing data persists until they collect.

function list(uint256 _tokenId, uint32 _price) external onlyWhitelisted {
// ...
listingsCounter++;
- s_listings[_tokenId] =
+ s_listings[listingsCounter] =
Listing({seller: msg.sender, price: _price, nft: address(this), tokenId: _tokenId, isActive: true});
emit NFT_Dealers_Listed(msg.sender, listingsCounter);
}

Support

FAQs

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

Give us feedback!