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

Unbacked Fee Accounting

Author Revealed upon completion

Unbacked Fee Accounting

Description

  • Normal behavior

    The contract collects protocol fees from NFT sales and tracks them using totalFeesCollected. These fees are expected to remain available in the contract and be withdrawable by the owner via withdrawFees().


  • Issue

    Fees are only tracked via a storage variable and are not explicitly segregated from the contract’s USDC balance. Seller payouts are executed from the same shared pool that includes fees, meaning the contract implicitly relies on perfect balance alignment. Any small reduction in the contract’s balance causes totalFeesCollected to exceed actual funds, leading to withdrawFees() reverting and permanently* locking protocol fees.

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; // @> Fees are only accounted, not reserved
​
amountToSeller += collateralToReturn;
​
usdc.safeTransfer(address(this), fees); // @> No-op (self-transfer)
usdc.safeTransfer(msg.sender, amountToSeller); // @> Pays seller from shared pool
}

Risk

Likelihood: MEDIUM

  • Occurs when the contract balance deviates slightly from expected accounting due to rounding, integration changes, or unexpected token behavior

  • Arises during normal operation as multiple trades and balance movements accumulate over time

Impact: HIGH

  • withdrawFees() can revert permanently* once fees are not fully backed

  • Protocol fees become locked and unclaimable, resulting in direct financial loss


Note on “permanent” revert:

The revert is considered permanent because once totalFeesCollected exceeds the contract’s actual token balance, there is no mechanism in the contract to restore consistency between accounting and available funds. The withdrawFees() function will continue to attempt transferring the full totalFeesCollected amount, which will always exceed the available balance and therefore always revert. Since the contract does not provide any way to reduce totalFeesCollected or recover the missing funds, fee withdrawals become indefinitely impossible.

Recommended Mitigation

Defensive check (fallback)

function withdrawFees() external onlyOwner {
+ require(
+ usdc.balanceOf(address(this)) >= totalFeesCollected,
+ "Insufficient balance for fees"
+ );

Ensure fees are never used to pay sellers.

uint256 fees = _calculateFees(listing.price);
+ uint256 sellerAmount = listing.price - fees;
​
- totalFeesCollected += fees;
- amountToSeller += collateralToReturn;
- usdc.safeTransfer(msg.sender, amountToSeller);
​
totalFeesCollected += fees;
- usdc.safeTransfer(address(this), fees); // remove self-transfer (no-op)
​
+ usdc.safeTransfer(msg.sender, sellerAmount + collateralToReturn);

Support

FAQs

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

Give us feedback!