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

Fee Self-Transfer Renders Protocol Fee Collection Insolvent

Author Revealed upon completion

Fee Self-Transfer Renders Protocol Fee Collection Insolvent

Description

  • The expected behavior is that protocol fees are retained in the contract when a seller settles a completed sale, tracked via totalFeesCollected, and later withdrawn by the owner through withdrawFees().

  • The issue is that collectUsdcFromSelling() executes usdc.safeTransfer(address(this), fees) — a transfer from the contract to itself — which is a no-op. totalFeesCollected is incremented as though fees were segregated, but no USDC is actually set aside. When the owner calls withdrawFees(), the payout draws from other users' locked collateral and uncollected sale proceeds, making the contract progressively insolvent.

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); // no-op: contract sends fees to itself
usdc.safeTransfer(msg.sender, amountToSeller);
}

Risk

Likelihood:

  • This triggers on every completed sale — collectUsdcFromSelling() is the only settlement path, and withdrawFees() is the normal revenue collection flow.

Impact:

  • The owner's fee withdrawal consumes USDC belonging to other users (collateral and uncollected proceeds), causing permanent loss of funds for later sellers and minters.

Proof of Concept

function testFeeInsolvency() public revealed {
vm.prank(owner);
nftDealers.whitelistWallet(userWithEvenMoreCash);
​
// User A mints + lists token 1
vm.startPrank(userWithCash);
usdc.approve(address(nftDealers), 20e6);
nftDealers.mintNft();
nftDealers.list(1, 1000e6);
vm.stopPrank();
​
// User B mints token 2 (20 USDC collateral locked)
vm.startPrank(userWithEvenMoreCash);
usdc.approve(address(nftDealers), type(uint256).max);
nftDealers.mintNft();
// User B buys token 1
nftDealers.buy(1);
vm.stopPrank();
​
// Contract holds: 20 (A collateral) + 20 (B collateral) + 1000 (sale) = 1040
​
// User A settles — self-transfer is no-op, gets 1010, contract left with 30
vm.prank(userWithCash);
nftDealers.collectUsdcFromSelling(1);
​
// Owner withdraws 10 USDC "fees" — actually taken from User B's collateral pool
vm.prank(owner);
nftDealers.withdrawFees();
​
// Only 20 USDC remains — exactly User B's collateral, zero margin
assertEq(usdc.balanceOf(address(nftDealers)), 20e6);
}

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];
​
totalFeesCollected += fees;
amountToSeller += collateralToReturn;
+ collateralForMinting[listing.tokenId] = 0;
+ delete s_listings[_listingId];
- 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!