NFT Dealers
First Flight #58
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

uint32 price field in Listing struct permanently prevents the HIGH (5%) fee tier from ever being charged, causing the protocol to systematically under-collect fees on all high-value sales

nahmstay

Description

The expected behavior of the progressive fee system is that NFT sales above 10,000 USDC are charged a 5% fee, sales between 1,000–10,000 USDC are charged 3%, and sales below 1,000 USDC are charged 1%. This is documented by the three fee constants and two thresholds in the contract.

What actually happens is that the price field in the Listing struct and the _price parameter in list() are both typed as uint32. The maximum value of uint32 is 4,294,967,295 — approximately 4,294 USDC with 6 decimal places. The HIGH_FEE_THRESHOLD is 10_000e6 = 10,000,000,000, which is more than twice uint32.max. It is therefore structurally impossible for any listing to ever reach the HIGH fee tier, regardless of what price a seller wants to set.

uint256 private constant LOW_FEE_THRESHOLD = 1000e6; // 1,000,000,000
uint256 private constant MID_FEE_THRESHOLD = 10_000e6; // 10,000,000,000 @> exceeds uint32 max (4,294,967,295)
​
struct Listing {
address seller;
uint32 price; // @> max value ~4294 USDC — can never reach HIGH_FEE_THRESHOLD
address nft;
uint256 tokenId;
bool isActive;
}
​
// @> _price is uint32 — Solidity will reject or silently truncate any value above uint32 max
function list(uint256 _tokenId, uint32 _price) external onlyWhitelisted { ... }

The same truncation applies when fees are calculated in collectUsdcFromSelling(), which reads listing.price as uint32 when passing it to _calculateFees().

Risk

Likelihood:

  • This affects every single sale in the protocol — there is no configuration or user action that can reach the HIGH tier

  • The issue is present from deployment and requires a contract upgrade to fix

Impact:

  • Every sale that a seller intends to list above ~4,294 USDC is either impossible to list (the caller's value is silently truncated to a different price) or capped at the MID (3%) fee tier instead of HIGH (5%)

  • The protocol permanently loses the difference between the 3% and 5% fee on all high-value sales — for a 10,000 USDC sale this is a 2% shortfall (200 USDC per sale)

  • Sellers who want to list above ~4,294 USDC cannot do so at their intended price

Proof of Concept

/**
* @notice M-01 PoC: Demonstrates that uint32.max price (the highest possible listing price)
* falls in the MID fee tier (3%), and that HIGH_FEE_THRESHOLD is unreachable.
*
* uint32.max = 4,294,967,295 (~4294 USDC)
* HIGH_FEE_THRESHOLD = 10,000,000,000 (~10000 USDC)
* 4,294,967,295 < 10,000,000,000 → HIGH tier can never apply
*/
function testUint32PriceCapsHighFeeTierUnreachable() public view { ... }

Run with:

forge test --mt testUint32PriceCapsHighFeeTierUnreachable -vv

Expected console output:

uint32 max price (raw): 4294967295
HIGH_FEE_THRESHOLD (raw): 10000000000
uint32 max < HIGH threshold: true
Fee actually charged at max price (MID 3%): 128849018
Fee that SHOULD be charged (HIGH 5%): 214748364
Protocol under-charges by: 85899346

Recommended Mitigation

Change Listing.price and all related function parameters from uint32 to uint256. The struct packs poorly with a uint32 price anyway since seller and nft are address (160-bit) and tokenId is uint256, so there is no meaningful gas benefit from the smaller type.

struct Listing {
address seller;
- uint32 price;
+ uint256 price;
address nft;
uint256 tokenId;
bool isActive;
}
​
- function list(uint256 _tokenId, uint32 _price) external onlyWhitelisted {
+ function list(uint256 _tokenId, uint256 _price) external onlyWhitelisted {
​
- function updatePrice(uint256 _listingId, uint32 _newPrice) external onlySeller(_listingId) {
+ function updatePrice(uint256 _listingId, uint256 _newPrice) external onlySeller(_listingId) {

This allows the _calculateFees() thresholds to operate as designed and enables sellers to list at any price without silent truncation.

Updates

Lead Judging Commences

rubik0n Lead Judge about 1 month ago
Submission Judgement Published
Validated
Assigned finding tags:

integer-overflow

cause - uint32 used

Support

FAQs

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

Give us feedback!