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

Double Claim Drain

Author Revealed upon completion

Root + Impact

Description

  • After selling an NFT, the seller calls collectUsdcFromSelling to get their money back (sale price minus fees plus the collateral they locked).

  • The function sends the USDC but never zeros out the listing price, so the seller can just call it again and drain the entire contract balance.

solidity

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); // still uses original 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); // sends money
@> // listing.price is never set to 0
@> // can be called again immediately
}

Risk

Likelihood:

  • Every time someone sells an NFT and calls this function, the price stays in storage

  • The require check only needs listing.isActive to be false, which it already is after the sale

  • Attacker calls the function multiple times in a loop until the contract is empty

Impact:

  • Attacker drains all USDC from the contract, stealing funds from other sellers

  • Legitimate sellers who sold NFTs can't collect their money anymore

  • Protocol becomes insolvent and unusable

Proof of Concept

solidity

// Contract has 2000 USDC total
// Alice sold her NFT for 1000 USDC
// listing.isActive = false (sale completed)
​
// Call #1
collectUsdcFromSelling(tokenId);
// Calculates: 1000 - 50 fees = 950, plus 20 collateral = 970 USDC
// Alice gets 970 USDC
// Contract now has 1030 USDC
// listing.price still = 1000 (BUG)
​
// Call #2
collectUsdcFromSelling(tokenId);
// Same calculation: 1000 - 50 = 950 (collateral already zeroed)
// Alice gets another 950 USDC
// Contract now has 80 USDC
​
// Call #3
collectUsdcFromSelling(tokenId);
// Alice gets final 80 USDC
// Contract drained to 0
// Other sellers can't collect their funds

Recommended Mitigation

diff

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];
​
+ s_listings[_listingId].price = 0;
+ collateralForMinting[listing.tokenId] = 0;
totalFeesCollected += fees;
amountToSeller += collateralToReturn;
- 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!