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

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

Author Revealed upon completion

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

Support

FAQs

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

Give us feedback!