NFT Dealers

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

`NFTDealers::updatePrice` allows prices under the `NFTDealers::MIN_PRICE`

Author Revealed upon completion

NFTDealers::updatePrice allows prices under the NFTDealers::MIN_PRICE

Description

NFTDealershas a constant MIN_PRICEwhich is set to 10e6. In the NFTDealers::listfunction, the _priceis required to be at least this amount. However, the updatePricefunction makes no such check, shown below.

// This function doesn't verify that _newPrice is larger than or equal to MIN_PRICE
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);
}

Risk

Likelihood:

  • This issue can be exploited by any NFT listing holder calling the updatePricefunction.

Impact:

  • Allows users to list for a price lower than the MIN_PRICE. This also means that users can list for a price low enough to have a fee of zero.

Proof of Concept

The following test shows the issue.

function testUpdatePriceAllowsPriceLowerThanMinPrice() public revealed {
uint256 tokenId = 1;
uint32 initialPrice = 1000e6;
uint32 newPrice = 1;
mintAndListNFTForTesting(tokenId, initialPrice);
vm.startPrank(userWithCash);
nftDealers.updatePrice(1, newPrice);
vm.stopPrank();
(,,,, bool isActive) = nftDealers.s_listings(1);
assertTrue(isActive);
(address seller, uint32 price,, uint256 listedTokenId,) = nftDealers.s_listings(1);
assertEq(seller, userWithCash);
assertEq(price, newPrice);
assertEq(listedTokenId, tokenId);
assertLt(price, nftDealers.MIN_PRICE());
}

Recommended Mitigation

This can be prevented by checking _newPriceagainst the MINPRICE

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 >= MIN_PRICE);
- require(_newPrice > 0, "Price must be greater than 0");
s_listings[_listingId].price = _newPrice;
emit NFT_Dealers_Price_Updated(_listingId, oldPrice, _newPrice);
}

Support

FAQs

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

Give us feedback!