Description
-
The marketplace is documented and coded as having a progressive fee schedule with three tiers: 1%, 3%, and 5%, based on the sale price. In the code, these tiers are controlled by LOW_FEE_THRESHOLD = 1000e6 and MID_FEE_THRESHOLD = 10_000e6, with anything above 10_000e6 charged at 5%.
-
The issue is that listing prices are stored and passed around as uint32 (Listing.price, list(uint32 _price), and updatePrice(uint32 _newPrice)). A uint32 can hold at most 4,294,967,295, which with 6-decimal USDC corresponds to only 4,294.967295 USDC. That is below the 10,000 USDC threshold required to ever reach the 5% fee tier, so the highest advertised fee tier is mathematically unreachable for real listings.
uint32 private constant LOW_FEE_BPS = 100;
uint32 private constant MID_FEE_BPS = 300;
uint32 private constant HIGH_FEE_BPS = 500;
uint256 private constant LOW_FEE_THRESHOLD = 1000e6;
uint256 private constant MID_FEE_THRESHOLD = 10_000e6;
struct Listing {
address seller;
uint32 price;
address nft;
uint256 tokenId;
bool isActive;
}
function list(uint256 _tokenId, uint32 _price) external onlyWhitelisted {
...
s_listings[_tokenId] =
Listing({seller: msg.sender, price: _price, nft: address(this), tokenId: _tokenId, isActive: true});
}
function updatePrice(uint256 _listingId, uint32 _newPrice) external onlySeller(_listingId) {
...
s_listings[_listingId].price = _newPrice;
}
function _calculateFees(uint256 _price) internal pure returns (uint256) {
if (_price <= LOW_FEE_THRESHOLD) {
return (_price * LOW_FEE_BPS) / MAX_BPS;
} else if (_price <= MID_FEE_THRESHOLD) {
return (_price * MID_FEE_BPS) / MAX_BPS;
}
return (_price * HIGH_FEE_BPS) / MAX_BPS;
}
``
Risk
Likelihood: Medium
-
This occurs on every marketplace deployment because the unreachable range is hardcoded into the contract through the combination of uint32 pricing and a 10_000e6 upper threshold.
-
This occurs for every real listing and every price update, because both list() and updatePrice() restrict prices to the same uint32 maximum.
Impact: High
-
The protocol cannot apply its advertised 5% fee tier, so fee collection is lower than intended for high-value sales.
-
The implementation diverges from the documented “progressive fee 1, 3 or 5%” model, which can mislead users, integrators, and fee/revenue forecasts.
Proof of Concept
-
Add import {console2} from "forge-std/console2.sol"; at the top of NFTDealersTest.t.sol.
-
Copy the code below to NFTDealersTest contract.
-
Run command forge test --mt testPriceStoredAsUint32MakesHighFeeTierUnreachable -vv.
function testPriceStoredAsUint32MakesHighFeeTierUnreachable() public revealed whitelisted {
uint256 tokenId = 1;
uint32 maxListablePrice = type(uint32).max;
uint256 maxListablePriceInUsdc = uint256(maxListablePrice) / 1e6;
uint256 highFeeThreshold = 10_000e6;
uint256 lowFeeThreshold = 1_000e6;
console2.log("type(uint32).max:", uint256(maxListablePrice));
console2.log("type(uint32).max in USDC (integer part):", maxListablePriceInUsdc);
console2.log("LOW_FEE_THRESHOLD:", lowFeeThreshold);
console2.log("MID/HIGH boundary threshold:", highFeeThreshold);
assertLt(uint256(maxListablePrice), highFeeThreshold, "BUG: uint32 max is below the 10,000 USDC threshold");
assertGt(uint256(maxListablePrice), lowFeeThreshold, "sanity: max uint32 is above 1,000 USDC");
mintNFTForTesting();
vm.prank(userWithCash);
nftDealers.list(tokenId, maxListablePrice);
(address seller, uint32 storedPrice,, uint256 listedTokenId, bool isActive) = nftDealers.s_listings(tokenId);
console2.log("Stored seller:", seller);
console2.log("Stored listing price:", uint256(storedPrice));
console2.log("Stored tokenId:", listedTokenId);
console2.log("Listing active flag:", isActive ? uint256(1) : uint256(0));
assertEq(storedPrice, maxListablePrice);
assertEq(listedTokenId, tokenId);
assertTrue(isActive);
vm.startPrank(userWithEvenMoreCash);
usdc.approve(address(nftDealers), uint256(maxListablePrice));
nftDealers.buy(tokenId);
vm.stopPrank();
uint256 actualFee = nftDealers.calculateFees(uint256(maxListablePrice));
uint256 expectedMidFee = (uint256(maxListablePrice) * 300) / 10_000;
uint256 expectedHighFee = (uint256(maxListablePrice) * 500) / 10_000;
console2.log("actualFee:", actualFee);
console2.log("expectedMidFee (3%):", expectedMidFee);
console2.log("expectedHighFee (5%):", expectedHighFee);
assertEq(actualFee, expectedMidFee, "BUG: maximum listable price still uses 3% fee");
assertTrue(actualFee != expectedHighFee, "BUG: 5% tier should be unreachable for uint32 listings");
vm.prank(userWithCash);
nftDealers.collectUsdcFromSelling(tokenId);
uint256 sellerFinalBalance = usdc.balanceOf(userWithCash);
uint256 expectedPayoutAt3Percent = uint256(maxListablePrice) + nftDealers.lockAmount() - expectedMidFee;
uint256 hypotheticalPayoutAt5Percent = uint256(maxListablePrice) + nftDealers.lockAmount() - expectedHighFee;
console2.log("sellerFinalBalance:", sellerFinalBalance);
console2.log("expectedPayoutAt3Percent:", expectedPayoutAt3Percent);
console2.log("hypotheticalPayoutAt5Percent:", hypotheticalPayoutAt5Percent);
assertEq(
sellerFinalBalance,
expectedPayoutAt3Percent,
"BUG: realized payout confirms only the 3% tier is reachable"
);
assertTrue(
sellerFinalBalance != hypotheticalPayoutAt5Percent,
"BUG: realized payout should never match an unreachable 5% tier"
);
}
Output:
[⠊] Compiling...
No files changed, compilation skipped
Ran 1 test for test/NFTDealersTest.t.sol:NFTDealersTest
[PASS] testPriceStoredAsUint32MakesHighFeeTierUnreachable() (gas: 384006)
Logs:
type(uint32).max: 4294967295
type(uint32).max in USDC (integer part): 4294
LOW_FEE_THRESHOLD: 1000000000
MID/HIGH boundary threshold: 10000000000
Stored seller: 0x22CdC71E987473D657FCe79C9C0C0B1A62148056
Stored listing price: 4294967295
Stored tokenId: 1
Listing active flag: 1
actualFee: 128849018
expectedMidFee (3%): 128849018
expectedHighFee (5%): 214748364
sellerFinalBalance: 4186118277
expectedPayoutAt3Percent: 4186118277
hypotheticalPayoutAt5Percent: 4100218931
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.73ms (577.60µs CPU time)
Ran 1 test suite in 12.36ms (2.73ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Recommended Mitigation
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");
listingsCounter++;
activeListingsCounter++;
s_listings[_tokenId] =
Listing({seller: msg.sender, price: _price, nft: address(this), tokenId: _tokenId, isActive: true});
emit NFT_Dealers_Listed(msg.sender, listingsCounter);
}
-function updatePrice(uint256 _listingId, uint32 _newPrice) external onlySeller(_listingId) {
+function updatePrice(uint256 _listingId, uint256 _newPrice) external onlySeller(_listingId) {
Listing memory listing = s_listings[_listingId];
uint256 oldPrice = listing.price;
if (!listing.isActive) revert ListingNotActive(_listingId);
require(_newPrice > 0, "Price must be greater than 0");
s_listings[_listingId].price = _newPrice;
emit NFT_Dealers_Price_Updated(_listingId, oldPrice, _newPrice);
}
