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

Seller can repeatedly claim the same sold listing and drain pooled USDC

Author Revealed upon completion

Root + Impact

Description

  • Normal behavior: after a listing is sold, the seller should be able to collect sale proceeds and returned collateral exactly once.

  • Specific issue: collectUsdcFromSelling does not track whether the payout was already claimed. The same seller can call it repeatedly for the same sold listing and receive funds multiple times.

// Root cause in the codebase with @> marks to highlight the relevant section
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:

  • Whenever a seller has a sold listing, they can call collectUsdcFromSelling again because the listing stays inactive and unclaimed.

  • As protocol balance grows from mints and buys, repeated calls keep succeeding until pooled funds are depleted.

Impact:

  • Repeated payout extraction drains USDC backed by other users' collateral and sale payments.

  • Protocol insolvency can follow, causing legitimate withdrawals and settlements to fail.

Proof of Concept

// Validated in codespace using:
// forge test --match-path test/SecurityValidationFindings.t.sol -vv
// Passing test: testFinding01_repeatCollectDrainsPool
​
vm.prank(seller);
nft.collectUsdcFromSelling(1);
​
vm.prank(seller);
nft.collectUsdcFromSelling(1);
​
assertEq(secondPayout, firstPayout); // second claim pays again

Recommended Mitigation

struct Listing {
address seller;
uint32 price;
address nft;
uint256 tokenId;
bool isActive;
+ bool wasSold;
+ bool proceedsClaimed;
}
​
function buy(uint256 _listingId) external {
...
s_listings[_listingId].isActive = false;
+ s_listings[_listingId].wasSold = true;
}
​
function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
- require(!listing.isActive, "Listing must be inactive to collect USDC");
+ require(listing.wasSold, "Listing not sold");
+ require(!listing.proceedsClaimed, "Already collected");
+
+ s_listings[_listingId].proceedsClaimed = true;
+ uint256 collateralToReturn = collateralForMinting[listing.tokenId];
+ collateralForMinting[listing.tokenId] = 0;
​
...
- usdc.safeTransfer(address(this), fees);
usdc.safeTransfer(msg.sender, amountToSeller);
}

Support

FAQs

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

Give us feedback!