NFT Dealers

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

H-5: Wrong Fee Threshold Values Cause Incorrect Fee Tier Assignment

Author Revealed upon completion

Description:

The constants LOW_FEE_THRESHOLD and MID_FEE_THRESHOLD are set as:

uint256 private constant LOW_FEE_THRESHOLD = 1000e6; // comment says "1.000 USDC" but value is 1,000 USDC
uint256 private constant MID_FEE_THRESHOLD = 10_000e6; // comment says "10.000 USDC" but value is 10,000 USDC

The comments use European decimal notation (period as thousands separator), suggesting the intended thresholds were 1 USDC and 10 USDC, but the actual values are 1,000 USDC and 10,000 USDC. This means a sale priced at 5 USDC (which should be in the mid tier at 3%) is charged only the low tier fee of 1%, and a sale at 5,000 USDC (which should be high tier at 5%) is charged only 3%.

Impact: The protocol systematically collects lower fees than intended across the entire price range, resulting in significant revenue loss for the protocol owner.

Recommended Mitigation: Clarify the intended thresholds with the protocol team and update the constants and comments to be consistent:

// Option A: If thresholds are meant to be 1 USDC and 10 USDC
uint256 private constant LOW_FEE_THRESHOLD = 1e6; // 1 USDC
uint256 private constant MID_FEE_THRESHOLD = 10e6; // 10 USDC
// Option B: If thresholds are meant to be 1,000 USDC and 10,000 USDC
uint256 private constant LOW_FEE_THRESHOLD = 1_000e6; // 1,000 USDC
uint256 private constant MID_FEE_THRESHOLD = 10_000e6; // 10,000 USDC

Proof of Concept (Forge):

function test_feeThreshold_wrongTierAssignment() public view {
// A price of 5 USDC should be MID tier (3%) but gets LOW tier (1%)
uint256 priceAt5USDC = 5e6;
uint256 actualFee = nftDealers.calculateFees(priceAt5USDC);
uint256 expectedMidFee = (priceAt5USDC * 300) / 10_000; // 3%
uint256 actualLowFee = (priceAt5USDC * 100) / 10_000; // 1%
// This will pass — confirming the wrong tier is applied
assertEq(actualFee, actualLowFee, "5 USDC gets 1% (LOW) fee instead of 3% (MID)");
assertNotEq(actualFee, expectedMidFee, "5 USDC should NOT get 3% fee but does");
}

Support

FAQs

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

Give us feedback!