NFT Dealers

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

Double claim collateral

Missing the collateral state updaet, Lead to Unlimited Collateral claiming

Description

  • The state of teh collateral should reset to zero after the seller collect it

  • After the seller collect the amountToSeller they state of teh collateral didn't update so they still 20 USDC that mean the seller can keep claiming it

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; //missing reset
usdc.safeTransfer(address(this), fees);
usdc.safeTransfer(msg.sender, amountToSeller);
}

Risk

Likelihood:

  • Reason 1: Every seller that have NFTs and sell it can call this and get the collateral

  • Reason 2: There is no reentrancy protection, so a user or malicious contract can call it multiple times in the same function

Impact:

  • Impact 1: An attacker can drain all the collateral in the contract, effectively stealing users’ funds

  • Impact 2: This causes huge financial damage to the protocol and its users, severely reducing trust in the

Proof of Concept

Scenario Setup:
User Alice owns an NFT with tokenId = 1.
collateralForMinting[1] = 20 USDC.
Alice lists and successfully sells her NFT.
The listing becomes inactive.
Action:
Alice calls:
collectUsdcFromSelling(1);
Observed Behavior:
Alice receives:
Sale amount + 20 USDC collateral
However, the state is NOT updated:
collateralForMinting[1] // still = 20 USDC
Alice can call again:
collectUsdcFromSelling(1);
She receives another 20 USDC
This can be repeated unlimited times.
Code Causing the Issue:
uint256 collateralToReturn = collateralForMinting[listing.tokenId];
amountToSeller += collateralToReturn; // ❌ missing reset
Expected Behavior:
After the first claim, collateral should be reset:
collateralForMinting[listing.tokenId] = 0;
The seller should only receive the collateral once.

Recommended Mitigation

- remove this code
+ collateralForMinting[listing.tokenId] = 0;
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!