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

collectUsdcFromSelling() does not clear collateralForMinting[tokenId], allowing collateral to be reused in later claims

themilenkov

Root + Impact

Description

Under normal behavior, the collateral locked during minting should only be returned once. After that collateral has been included in a seller payout, the token’s collateral accounting should be cleared so the same amount cannot be counted again in future flows.

In the current implementation, collectUsdcFromSelling() reads collateralForMinting[listing.tokenId] and adds it to the seller payout, but it never zeroes that mapping entry afterward. As a result, the same collateral remains recorded in storage even after being paid out, and can be counted again in subsequent invalid or repeated claims.

function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
...
uint256 collateralToReturn = collateralForMinting[listing.tokenId];
// @> collateral is read and added to payout
​
totalFeesCollected += fees;
amountToSeller += collateralToReturn;
usdc.safeTransfer(msg.sender, amountToSeller);
​
// @> collateralForMinting[listing.tokenId] is never set to 0
}

Risk

Likelihood:

  • The bug occurs whenever collectUsdcFromSelling() is called for a token whose collateral is still recorded in collateralForMinting.

  • The issue is easy to trigger because the function always reads the current collateral value and never clears it after payout.

Impact:

  • The same collateral can be included in multiple claims.

  • This amplifies the impact of repeated-claim and invalid-claim paths by adding extra value to each payout.

  • The contract’s pooled USDC can be drained faster than intended.

Proof of Concept

Paste this inside NFTDealersTest.t.sol:

function testCollectUsdcFromSellingDoesNotClearCollateral() public revealed {
uint256 tokenId = 1;
uint32 nftPrice = 1000e6;
​
mintAndListNFTForTesting(tokenId, nftPrice);
​
vm.startBroadcast(userWithEvenMoreCash);
usdc.approve(address(nftDealers), nftPrice);
nftDealers.buy(tokenId);
vm.stopBroadcast();
​
uint256 collateralBefore = nftDealers.collateralForMinting(tokenId);
assertEq(collateralBefore, nftDealers.lockAmount());
​
vm.prank(userWithCash);
nftDealers.collectUsdcFromSelling(tokenId);
​
uint256 collateralAfter = nftDealers.collateralForMinting(tokenId);
​
// Collateral remains recorded even after it was included in payout.
assertEq(collateralAfter, nftDealers.lockAmount());
}

Recommended Mitigation

Clear the collateral state before transferring funds out, so each token’s collateral can only be returned once.

function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
- Listing memory listing = s_listings[_listingId];
- require(!listing.isActive, "Listing must be inactive to collect USDC");
+ 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];
​
+ 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!