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

Re-listing a sold token overwrites original seller's uncollected proceeds

ramilmustafin33

Description

  • After a successful sale via buy(), the original seller is expected to call collectUsdcFromSelling to receive their price - fees + collateral. The function uses onlySeller(_listingId) which checks s_listings[_listingId].seller == msg.sender.

  • Listings are keyed by tokenId (s_listings[_tokenId]). When the buyer becomes the new owner and calls list() for the same tokenId, the new listing overwrites s_listings[tokenId] entirely — including the seller field. The original seller's address is now replaced with the buyer's address, permanently failing the onlySeller check and locking the original seller's sale proceeds and collateral in the contract forever. The only guard in list() is isActive == false, which is already satisfied after a sale.

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"); // no check for uncollected proceeds
​
listingsCounter++;
activeListingsCounter++;
​
@> s_listings[_tokenId] = // overwrites original seller's listing
Listing({seller: msg.sender, price: _price, nft: address(this), tokenId: _tokenId, isActive: true});
emit NFT_Dealers_Listed(msg.sender, listingsCounter);
}
​
modifier onlySeller(uint256 _listingId) {
@> require(s_listings[_listingId].seller == msg.sender, "Only seller can call this function");
// original seller fails here after overwrite
_;
}

Risk

Likelihood:

  • The buyer calls list() immediately after purchase — a natural marketplace action for reselling an NFT.

  • No special conditions required: the buyer just needs to be whitelisted (which they are, since they had to be whitelisted to interact with the contract).

Impact:

  • Original seller permanently loses their sale proceeds (price - fees) and minting collateral (lockAmount).

  • Funds remain locked in the contract with no recovery path.

Proof of Concept

Paste this function inside NFTDealersTest in test/NFTDealersTest.t.sol and run:
forge test --match-test testPoC_RelistOverwritesOriginalSellerProceeds -vvvv

function testPoC_RelistOverwritesOriginalSellerProceeds() public {
address alice = makeAddr("alice");
address bob = makeAddr("bob");
​
usdc.mint(alice, 20e6);
usdc.mint(bob, 100e6); // to buy alice's NFT
​
vm.prank(owner);
nftDealers.revealCollection();
vm.startPrank(owner);
nftDealers.whitelistWallet(alice);
nftDealers.whitelistWallet(bob); // bob needs whitelist to re-list
vm.stopPrank();
​
// Alice mints (tokenId = 1) and lists at 100 USDC
vm.startPrank(alice);
usdc.approve(address(nftDealers), 20e6);
nftDealers.mintNft();
nftDealers.list(1, uint32(100e6));
vm.stopPrank();
​
// Bob buys Alice's NFT
vm.startPrank(bob);
usdc.approve(address(nftDealers), 100e6);
nftDealers.buy(1);
vm.stopPrank();
​
// isActive = false, alice has not yet collected her proceeds
// Bob immediately re-lists the same tokenId — overwrites s_listings[1].seller
vm.prank(bob);
nftDealers.list(1, uint32(50e6)); // s_listings[1].seller = bob now
​
// Alice tries to collect her legitimate sale proceeds
vm.prank(alice);
vm.expectRevert("Only seller can call this function");
nftDealers.collectUsdcFromSelling(1);
​
// Alice's proceeds (99e6) and collateral (20e6) are permanently locked
assertEq(usdc.balanceOf(alice), 0);
assertGt(usdc.balanceOf(address(nftDealers)), 0);
}

Recommended Mitigation

- function list(uint256 _tokenId, uint32 _price) external onlyWhitelisted {
+ function list(uint256 _tokenId, uint256 _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");
+ require(s_listings[_tokenId].isActive == false && s_listings[_tokenId].price == 0, "NFT has pending claim or is listed");
require(_price > 0, "Price must be greater than 0");
​
listingsCounter++;
activeListingsCounter++;
​
s_listings[_tokenId] =
Listing({seller: msg.sender, price: _price, nft: address(this), tokenId: _tokenId, isActive: true});
emit NFT_Dealers_Listed(msg.sender, listingsCounter);
}
Updates

Lead Judging Commences

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

locked-funds-on-resell

rapid buy and list locks fund from 1st seller

Support

FAQs

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

Give us feedback!