NFT Dealers

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

[H-3] Unsafe downcasting of listings' prices to `uint32` causes a price limitation

Author Revealed upon completion

Unsafe downcasting of listings' prices to uint32 causes a price limitation

Description

In the definition of the Listing struct, the type of price is uint32. This means that the price of a listing can never exceed 2**32-1 (4294967295), which is equivalent to 4294.967295 USDC. This is a critical issue because the app is supposed to allow higher prices than 10,000 USDC, which is the MID_FEE_THRESHOLD. Moreover, if the frontend is not handling type casting correctly, the listing price can be silently truncated, resulting in a loss of funds if the user does not notice it.

struct Listing {
address seller;
@> uint32 price;
address nft;
uint256 tokenId;
bool isActive;
}

Risk

Likelihood:

  • It will happen everytime a user wants to list a NFT at a higher price than 4295 USDC.

Impact:

  • Price is limited to approximatively 4295 USDC

  • Frontend can silently truncate user's listing price

  • The protocol will generate fewer fees than expected

Proof of Concept

Actors:

  • Seller: A normal user minting an NFT and trying to sell it

Proof of Code:

function testPriceLimit() public {
// Seller has to be whitelisted and the collection revealed
vm.startPrank(owner);
nftDealers.whitelistWallet(seller);
nftDealers.revealCollection();
vm.stopPrank();
// Seller mints an NFT and tries to list it at a price higher than 2**32-1
uint256 tokenId = 1;
uint256 tokenPrice = 4295967296; // 4295.967296 USDC
vm.startPrank(seller);
usdc.approve(address(nftDealers), 20e6);
nftDealers.mintNft();
// We need to cast the price to uint32 because it's the type of the price parameter
uint32 castedTokenPrice = uint32(tokenPrice);
nftDealers.list(tokenId, castedTokenPrice);
vm.stopPrank();
// The NFT is listed at a price of 1 USDC instead of 4295.967296 USDC
(, uint32 storedPrice, , , ) = nftDealers.s_listings(tokenId);
vm.assertEq(storedPrice, 1e6);
}

Recommended Mitigation

Use uint256 for the price of the Listing object.

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) {

Support

FAQs

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

Give us feedback!