NFT Dealers

First Flight #58
Beginner FriendlyFoundry
100 EXP
Submission Details
Impact: medium
Likelihood: high

calculateFees() Function Is Publicly Accessible - Allows Users to Game Fee Threshold System

Author Revealed upon completion

Root cause is that calculateFees() is a public function that allows anyone to query exact fee amounts before listing. This enables users to precisely game the fee threshold boundaries (1000 USDC and 10000 USDC) to minimize fees. The function was intended for testing but was left in production.

Impact: Users can enumerate the entire fee structure and optimize listings to avoid higher fee tiers. Protocol loses revenue as users cluster just below fee thresholds. Internal fee calculation logic is exposed, reducing protocol opacity.

Description

  • The NFT Dealers protocol uses a progressive fee structure: 1% for prices ≤1000 USDC, 3% for prices ≤10000 USDC, and 5% for prices >10000 USDC. The calculateFees() function was added to help with testing and debugging.

  • However, calculateFees() was left public in production. This allows any user to query exact fee amounts before listing, enabling them to game the fee threshold system. Users can find the optimal price points that minimize fees while maximizing proceeds. The function comment even states it "must be removed before production deployment" but was forgotten.

// src/NFTDealers.sol::calculateFees()
@> function calculateFees(uint256 price) external pure returns (uint256) {
@> // for testing purposes, we want to be able to call this function directly
@> // must be removed before production deployment
@> return _calculateFees(price);
@> }
function _calculateFees(uint256 _price) internal pure returns (uint256) {
if (_price <= LOW_FEE_THRESHOLD) {
return (_price * LOW_FEE_BPS) / MAX_BPS; // 1%
} else if (_price <= MID_FEE_THRESHOLD) {
return (_price * MID_FEE_BPS) / MAX_BPS; // 3%
}
return (_price * HIGH_FEE_BPS) / MAX_BPS; // 5%
}

Risk

Likelihood:

  • This occurs whenever users call calculateFees() before listing to optimize their price

  • The function is publicly accessible with no restrictions - anyone can call it

Impact:

  • Users can game fee threshold system to minimize protocol fees

  • Protocol loses revenue as users cluster just below fee boundaries

Proof of Concept

The following PoC demonstrates that anyone can call calculateFees() to query exact fee amounts before listing. This allows users to find optimal price points that minimize fees.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.30;
import { Test } from "forge-std/Test.sol";
import { NFTDealers } from "src/NFTDealers.sol";
import { MockUSDC } from "src/MockUSDC.sol";
contract M01_PoC is Test {
NFTDealers public nftDealers;
MockUSDC public usdc;
address owner = makeAddr("owner");
function setUp() public {
usdc = new MockUSDC();
nftDealers = new NFTDealers(owner, address(usdc), "NFT Dealers", "NFTD", "ipfs://image", 20 * 1e6);
}
function test_CalculateFeesPublicAccess() public {
// Anyone can query fees before listing
uint256 fee1 = nftDealers.calculateFees(1000 * 1e6);
uint256 fee2 = nftDealers.calculateFees(1001 * 1e6);
uint256 fee3 = nftDealers.calculateFees(10000 * 1e6);
console.log("Fee at 1000 USDC:", fee1);
console.log("Fee at 1001 USDC:", fee2);
console.log("Fee at 10000 USDC:", fee3);
console.log("VULNERABILITY: Fee structure can be enumerated");
console.log("Users can game threshold boundaries");
}
}

Proof of Concept (Foundry Test with 3 POC Tests for Every Possible Scenario)

The comprehensive test suite below validates the vulnerability across three scenarios: (1) Users can calculate exact fees before listing, (2) Fee structure information leak allows threshold detection, (3) Function should be removed before production. All tests pass and confirm the vulnerability.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.30;
/**
* ============================================================
* POC-M01: calculateFees() Public Function Should Be Removed
* Allows users to game fee threshold system
* Severity : MEDIUM
* Contract : NFTDealers.sol
* Function : calculateFees()
* Author: Sudan249 AKA 0xAljzoli
* ============================================================
*
* VULNERABLE CODE:
*
* function calculateFees(uint256 price) external pure returns (uint256) {
* // for testing purposes, we want to be able to call this function directly
* // must be removed before production deployment
* return _calculateFees(price);
* }
*
* IMPACT:
* - Users can calculate exact fees before listing
* - Allows precise gaming of fee threshold boundaries
* - Reveals internal fee structure (minor info leak)
* - Increases attack surface (public function)
* - Comment says remove before production but easy to forget
*
* FIX:
* - Remove function before production deployment
* - OR keep for transparency but document properly
* - OR make internal only
*/
import { Test } from "forge-std/Test.sol";
import { console } from "forge-std/console.sol";
import "./AuditBase.sol";
contract POC_M01_CalculateFeesPublicExposure is AuditBase {
// ------------------------------------------------------------------
// POC A: User Can Calculate Exact Fees Before Listing
// ------------------------------------------------------------------
function test_M01_A_userCanCalculateFeesBeforeListing() public {
uint256 price1 = 1000 * 1e6;
uint256 price2 = 1001 * 1e6;
uint256 price3 = 10_000 * 1e6;
uint256 price4 = 10_001 * 1e6;
uint256 fee1 = nftDealers.calculateFees(price1);
uint256 fee2 = nftDealers.calculateFees(price2);
uint256 fee3 = nftDealers.calculateFees(price3);
uint256 fee4 = nftDealers.calculateFees(price4);
console.log("Price: 1000 USDC, Fee:", fee1, "bps: 1%");
console.log("Price: 1001 USDC, Fee:", fee2, "bps: 3%");
console.log("Price: 10000 USDC, Fee:", fee3, "bps: 3%");
console.log("Price: 10001 USDC, Fee:", fee4, "bps: 5%");
console.log("VULNERABILITY: Users can game the fee system");
console.log("FIX: Remove calculateFees() or make internal");
}
// ------------------------------------------------------------------
// POC B: Fee Structure Information Leak
// ------------------------------------------------------------------
function test_M01_B_feeStructureInfoLeak() public {
uint256 lowThreshold = 1000 * 1e6;
uint256 midThreshold = 10_000 * 1e6;
uint256 feeBelowLow = nftDealers.calculateFees(lowThreshold);
uint256 feeAboveLow = nftDealers.calculateFees(lowThreshold + 1);
uint256 feeBelowMid = nftDealers.calculateFees(midThreshold);
uint256 feeAboveMid = nftDealers.calculateFees(midThreshold + 1);
console.log("Fee at low threshold:", feeBelowLow);
console.log("Fee above low threshold:", feeAboveLow);
console.log("Fee at mid threshold:", feeBelowMid);
console.log("Fee above mid threshold:", feeAboveMid);
if (feeAboveLow > feeBelowLow * 2) {
console.log("THRESHOLD DETECTED: Fee jumps at", lowThreshold);
}
if (feeAboveMid > feeBelowMid * 15 / 10) {
console.log("THRESHOLD DETECTED: Fee jumps at", midThreshold);
}
console.log("INFO LEAK: Fee structure can be enumerated");
}
// ------------------------------------------------------------------
// POC C: Function Should Be Removed Before Production
// ------------------------------------------------------------------
function test_M01_C_functionShouldBeRemoved() public view {
console.log("MEDIUM ISSUE: calculateFees() is public");
console.log("Comment in code says 'must be removed before production'");
console.log("Should be removed or properly documented");
}
}

Recommended Mitigation

The fix removes the calculateFees() function entirely or changes it to internal only. This prevents users from querying fee amounts before listing.

- function calculateFees(uint256 price) external pure returns (uint256) {
- // for testing purposes, we want to be able to call this function directly
- // must be removed before production deployment
- return _calculateFees(price);
- }
+ // ✅ Removed public calculateFees() function
+ // Fee calculation is internal only - users cannot query before listing

Mitigation Explanation: The fix addresses the root cause by: (1) Removing the public calculateFees() function entirely, preventing users from querying exact fee amounts before listing, (2) Keeping _calculateFees() as internal only for use within the contract, (3) This prevents users from gaming the fee threshold system by finding optimal price points, (4) Alternatively, if transparency is desired, the function could be kept but the comment should be updated to reflect intentional design rather than "must be removed", (5) Removing test-only functions before production is a security best practice that reduces attack surface.

Support

FAQs

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

Give us feedback!