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

`collectUsdcFromSelling()` Authorizes Claims Using Mutable `listing.seller`, Blocking The Rightful Seller After Resale

Author Revealed upon completion

collectUsdcFromSelling() Authorizes Claims Using Mutable listing.seller, Blocking The Rightful Seller After Resale

Description

The intended flow is that after each successful sale, the account that just sold the NFT should be the one entitled to collect the sale proceeds for that sale.

In collectUsdcFromSelling(), access control is enforced with onlySeller(_listingId), which reads s_listings[_listingId].seller. However, listings are keyed by token ID and reused across future resales. After a buyer purchases an NFT and re-lists it, the same storage slot is reused for the new listing. This makes the authorization logic unreliable because entitlement to proceeds is derived from mutable listing state rather than from immutable sale records. In practice, this can leave the rightful seller unable to collect proceeds after a later resale, while another account associated with stale listing state can interfere with settlement.

function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
//@audit - High - Unreliable checking mechanism (modifier and require). If the buyer re-list their NFT right after buying, both these two checks will fail.
Listing memory listing = s_listings[_listingId];
require(!listing.isActive, "Listing must be inactive to collect USDC");
​
uint256 fees = _calculateFees(listing.price);
uint256 amountToSeller = listing.price - fees;
uint256 collateralToReturn = collateralForMinting[listing.tokenId];
​
totalFeesCollected += fees;
amountToSeller += collateralToReturn;
usdc.safeTransfer(address(this), fees);
usdc.safeTransfer(msg.sender, amountToSeller);
}

Risk

Likelihood:

  • The issue occurs during normal secondary-market usage whenever the same token is sold and then re-listed, because the listing record for that token is overwritten and reused.

  • The protocol explicitly supports resale, so this stale-state path is part of the expected lifecycle rather than an edge case.

Impact:

  • The seller who should receive proceeds from a completed resale can be unable to claim them.

  • Settlement for secondary sales becomes unreliable because claim authorization is based on mutable listing storage instead of a dedicated sale payout record.

Proof of Concept

The following PoC can be used in test/NFTDealersTest.t.sol to demonstrate that the rightful seller cannot claim their proceeds after a resale:

function testOriginalSellerCantClaim() public revealed whitelisted {
vm.prank(owner);
nftDealers.whitelistWallet(userWithEvenMoreCash);
//Firt user mint and list NFT
vm.startPrank(userWithCash);
usdc.approve(address(nftDealers), 20e6);
nftDealers.mintNft();
nftDealers.list(1, 1000e6);
vm.stopPrank();
​
//Second user buys NFT, then re-list
vm.startPrank(userWithEvenMoreCash);
usdc.approve(address(nftDealers), 10_000e6);
nftDealers.buy(1);
nftDealers.list(1, 2000e6);
vm.stopPrank();
//First user collect funds
vm.expectRevert();
vm.prank(userWithCash);
nftDealers.collectUsdcFromSelling(1);
}

Recommended Mitigation

Do not authorize claims from the mutable listing record. Instead, create dedicated sale-proceeds accounting at the moment of purchase and store the claimable amount in a double mapping keyed by listingId and seller address. This preserves each seller's entitlement even when the same NFT is re-listed and sold again before an earlier seller withdraws.

+mapping(uint256 => mapping(address => uint256)) public claimableAmount;
​
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, "");
​
+ claimableAmount[_listingId][listing.seller] += listing.price;
s_listings[_listingId].isActive = false;
​
emit NFT_Dealers_Sold(msg.sender, listing.price);
}
​
-function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
+function collectUsdcFromSelling(uint256 _listingId) external {
Listing memory listing = s_listings[_listingId];
+ require(claimableAmount[_listingId][msg.sender] > 0, "No claimable proceeds");
​
+ uint256 saleAmount = claimableAmount[_listingId][msg.sender];
+ uint256 fees = _calculateFees(saleAmount);
+ uint256 amountToSeller = saleAmount - fees;
uint256 collateralToReturn = collateralForMinting[listing.tokenId];
​
+ claimableAmount[_listingId][msg.sender] = 0;
totalFeesCollected += fees;
amountToSeller += collateralToReturn;
usdc.safeTransfer(msg.sender, amountToSeller);
}

Support

FAQs

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

Give us feedback!