NFT Dealers

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

`uint32 price` type caps maximum NFT listing price at ~4294 USDC, making the `HIGH_FEE` (5%) tier permanently unreachable dead code

Author Revealed upon completion

Root + Impact

Description

The price field in the Listing struct (L56) and the _price parameter in list() (L127) and updatePrice() (L185) are typed as uint32. The maximum uint32 value is 4,294,967,295, which with USDC's 6 decimals equals ~4294.97 USDC. However, the fee calculation function _calculateFees() (L229-236) defines three fee tiers using uint256 thresholds:

Constant Line Value USDC Equivalent
LOW_FEE_THRESHOLD L28 1000e6 (1,000,000,000) 1,000 USDC
MID_FEE_THRESHOLD L29 10_000e6 (10,000,000,000) 10,000 USDC
type(uint32).max -- 4,294,967,295 ~4,294 USDC

Since type(uint32).max (4,294,967,295) < MID_FEE_THRESHOLD (10,000,000,000), the listing.price value passed to _calculateFees() at L175 can never exceed MID_FEE_THRESHOLD. This makes the third branch of _calculateFees() at L235 -- the HIGH_FEE_BPS (5%) tier -- permanently unreachable dead code:

function _calculateFees(uint256 _price) internal pure returns (uint256) { // @audit L229: takes uint256
if (_price <= LOW_FEE_THRESHOLD) { // @audit L230: <= 1000 USDC
return (_price * LOW_FEE_BPS) / MAX_BPS; // @audit L231: 1% fee
} else if (_price <= MID_FEE_THRESHOLD) { // @audit L232: <= 10,000 USDC
return (_price * MID_FEE_BPS) / MAX_BPS; // @audit L233: 3% fee
}
return (_price * HIGH_FEE_BPS) / MAX_BPS; // @audit L235: 5% fee -- UNREACHABLE
}

The root cause is the type mismatch: the Listing struct stores price as uint32 (L56), and both list() (L127) and updatePrice() (L185) accept uint32 parameters, but the fee thresholds are defined as uint256 with values exceeding uint32 range. When listing.price (uint32) is passed to _calculateFees(uint256) at L175, it is implicitly upcast to uint256, but its value can never exceed 4,294,967,295.

The three constrained locations:

struct Listing {
address seller;
uint32 price; // @audit L56: max ~4294 USDC, below HIGH_FEE threshold of 10,000 USDC
address nft;
uint256 tokenId;
bool isActive;
}
function list(uint256 _tokenId, uint32 _price) external onlyWhitelisted { // @audit L127: uint32 _price
...
}
function updatePrice(uint256 _listingId, uint32 _newPrice) external onlySeller(_listingId) { // @audit L185: uint32 _newPrice
...
}

This produces two impacts:

  1. Broken fee structure: The protocol's HIGH_FEE_BPS (500, L26) and MID_FEE_THRESHOLD (10,000e6, L29) are dead code. The MID_FEE tier is also only partially reachable (covers ~1000-4294 USDC instead of the designed 1000-10,000 USDC range).

  2. Capped marketplace: NFTs cannot be listed above ~4294 USDC, preventing high-value sales on the marketplace entirely.

Risk

Likelihood:

  • The uint32 constraint is hardcoded in the Listing struct (L56) and both listing functions (L127, L185) -- it affects 100% of listings

  • The HIGH_FEE tier is unreachable in every possible scenario -- there is no code path that can trigger the 5% fee

  • The MID_FEE tier is partially unreachable: prices between ~4295 and 10,000 USDC cannot exist

Impact:

  • Protocol collects 3% instead of the designed 5% on the highest-value sales -- at the maximum price of ~4294 USDC, the protocol loses ~85.9 USDC in fees per sale (HIGH_FEE would collect ~214.7 USDC vs MID_FEE's ~128.8 USDC)

  • The marketplace cannot support NFTs priced above ~4294 USDC, severely limiting the protocol's utility for high-value collections

  • The HIGH_FEE_BPS constant (L26), MID_FEE_THRESHOLD constant (L29), and the third branch of _calculateFees() (L235) are all dead code

Proof of Concept

Two Foundry tests demonstrate both impacts. Run with forge test --match-contract M03PoCTest -vv:

// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.34;
import {Test, console} from "forge-std/Test.sol";
import {NFTDealers} from "../src/NFTDealers.sol";
import {MockUSDC} from "../src/MockUSDC.sol";
contract M03PoCTest is Test {
NFTDealers public nftDealers;
MockUSDC public usdc;
address public owner = makeAddr("owner");
address public seller = makeAddr("seller");
uint256 constant LOCK_AMOUNT = 20e6; // 20 USDC
function setUp() public {
usdc = new MockUSDC();
nftDealers = new NFTDealers(owner, address(usdc), "NFTDealers", "NFTD", "ipfs://test/", LOCK_AMOUNT);
vm.startPrank(owner);
nftDealers.revealCollection();
nftDealers.whitelistWallet(seller);
vm.stopPrank();
usdc.mint(seller, 10000e6);
vm.prank(seller);
usdc.approve(address(nftDealers), type(uint256).max);
}
function test_highFeeTierUnreachable() public view {
// --- Demonstrate uint32 max is below MID_FEE_THRESHOLD ---
uint256 maxUint32 = uint256(type(uint32).max); // 4,294,967,295
uint256 midFeeThreshold = 10_000e6; // 10,000,000,000
console.log("uint32 max: ", maxUint32);
console.log("MID_FEE_THRESHOLD: ", midFeeThreshold);
console.log("uint32 max in USDC:", maxUint32 / 1e6);
// uint32 max (~4294 USDC) < MID_FEE_THRESHOLD (10,000 USDC)
assertLt(maxUint32, midFeeThreshold, "uint32 max is below MID_FEE_THRESHOLD");
// --- Fee at the maximum possible listing price is MID (3%), never HIGH (5%) ---
uint256 feeAtMaxPrice = nftDealers.calculateFees(maxUint32);
uint256 expectedMidFee = (maxUint32 * 300) / 10_000; // 3% (MID_FEE_BPS)
uint256 expectedHighFee = (maxUint32 * 500) / 10_000; // 5% (HIGH_FEE_BPS)
assertEq(feeAtMaxPrice, expectedMidFee, "Max uint32 price triggers MID_FEE (3%)");
assertTrue(feeAtMaxPrice != expectedHighFee, "HIGH_FEE (5%) is never triggered");
console.log("Fee at max price (3%): ", feeAtMaxPrice);
console.log("Would-be HIGH_FEE (5%): ", expectedHighFee);
console.log("Lost fee revenue per sale: ", expectedHighFee - feeAtMaxPrice);
}
function test_cannotListAbove4294USDC() public {
// Mint an NFT
vm.prank(seller);
nftDealers.mintNft(); // tokenId 1
// List at the maximum uint32 price (~4294 USDC) -- succeeds
vm.prank(seller);
nftDealers.list(1, type(uint32).max);
(address listedSeller, uint32 listedPrice,,, bool isActive) = nftDealers.s_listings(1);
assertEq(listedSeller, seller);
assertEq(listedPrice, type(uint32).max);
assertTrue(isActive);
console.log("Max listing price (USDC):", uint256(listedPrice) / 1e6);
console.log("NFTs worth >4294 USDC cannot be listed on this marketplace");
// There is no way to pass a value > type(uint32).max to list() or updatePrice()
// because the parameter type is uint32 -- Solidity enforces this at the ABI level
}
}

Recommended Mitigation

Change the price type from uint32 to uint256 in the Listing struct and both function parameters. This removes the artificial cap, enables the full fee tier structure, and allows high-value NFT sales.

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 {
require(_price >= MIN_PRICE, "Price must be at least 1 USDC");
require(ownerOf(_tokenId) == msg.sender, "Not owner of NFT");
require(s_listings[_tokenId].isActive == false, "NFT is already listed");
require(_price > 0, "Price must be greater than 0");
...
}
-function updatePrice(uint256 _listingId, uint32 _newPrice) external onlySeller(_listingId) {
+function updatePrice(uint256 _listingId, uint256 _newPrice) external onlySeller(_listingId) {
...
}

This fix:

  1. Enables the full fee tier structure: LOW_FEE (1%) up to 1,000 USDC, MID_FEE (3%) up to 10,000 USDC, HIGH_FEE (5%) above 10,000 USDC

  2. Removes the ~4294 USDC price cap, allowing high-value NFT sales

  3. Aligns with _calculateFees() (L229) which already accepts uint256

  4. Is compatible with all existing downstream code: buy() (L148), collectUsdcFromSelling() (L175-176), and events (L18-19) all already use uint256 for price-related values

Support

FAQs

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

Give us feedback!