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

collectUsdcFromSelling() transfers fees to itself and never resets collateral, allowing a seller to drain the entire contract

Author Revealed upon completion

Root + Impact

Description

  • After a listing is sold, the seller calls collectUsdcFromSelling() to receive their sale proceeds.

  • collateralForMinting[listing.tokenId] — deposited by the original minter — is unconditionally bundled into the seller's payout. Since NFTs can change hands, the seller and original minter are not the same address, causing the minter to permanently lose their locked collateral. Additionally, fees are sent to address(this) instead of owner, leaving them stuck in the contract.

function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
...
@> uint256 collateralToReturn = collateralForMinting[listing.tokenId];
...
@> amountToSeller += collateralToReturn; // seller receives minter's collateral
@> usdc.safeTransfer(address(this), fees); // fee stuck — transfer to self
usdc.safeTransfer(msg.sender, amountToSeller);
}

Risk

Likelihood:

  • Every secondary sale triggers this — the collateral theft occurs on every resell where the seller is not the original minter.

  • The original minter has no way to recover their collateral once paid out to the seller.

Impact:

  • Original minters permanently lose their locked collateral on every resell.

  • Protocol fees are stuck in the contract and unwithdrawable by the owner.

Proof of Concept

  1. Bob, Charlie, and Alice each mint an NFT, locking 20 USDC collateral each. Contract holds 60 USDC.

  2. Alice lists her NFT at 100 USDC. Bob buys it — contract now holds 160 USDC.

  3. Alice calls collectUsdcFromSelling(3) and receives 119 USDC (99 sale proceeds + 20 collateral).

  4. Contract is left with 41 USDC — Bob and Charlie's 40 USDC collateral is permanently stuck with no recovery path, and 1 USDC fee is stuck from the self-transfer.

function test_drainCollateral() public {
vm.prank(bob); nftDealers.mintNft(); // tokenId 1, collateral 20 USDC
vm.prank(charlie); nftDealers.mintNft(); // tokenId 2, collateral 20 USDC
vm.prank(alice); nftDealers.mintNft(); // tokenId 3, collateral 20 USDC
vm.prank(alice); nftDealers.list(3, 100e6);
vm.prank(bob); nftDealers.buy(3); // contract holds 160 USDC
​
uint256 aliceBefore = usdc.balanceOf(alice);
vm.prank(alice);
nftDealers.collectUsdcFromSelling(3);
​
assertEq(usdc.balanceOf(alice) - aliceBefore, 119e6); // got 20 USDC extra
assertEq(usdc.balanceOf(address(nftDealers)), 41e6); // bob & charlie funds stuck
}

Recommended Mitigation

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);
+ usdc.safeTransfer(owner, fees);
+ usdc.safeTransfer(msg.sender, amountToSeller);

Track the original minter address separately and allow them to claim their collateral independently after the NFT is sold.

Support

FAQs

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

Give us feedback!