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

Double Claim Drain

jhongamersx

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);
}
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!