NFT Dealers

Root + Impact

Description

Normal behavior dictates that users should be able to list their NFTs for any price they desire, and the smart contract should securely store and enforce that exact price.

The specific issue is that the Listing struct and the list()/updatePrice() functions use a uint32 data type to store the NFT price. Because the USDC token has 6 decimals, the maximum representable value in a uint32 is 4,294,967,295 (equivalent to ~$4,294.96 USDC). If a user attempts to list an NFT for a price higher than this threshold (e.g., 5,000 USDC) and a frontend or script explicitly downcasts the uint256 value to uint32 to interact with the contract, Solidity 0.8+ will not revert. Instead, it silently truncates the most significant bits.

Solidity
// @> The struct limits the price to a uint32
struct Listing {
address seller;
uint32 price;
address nft;
uint256 tokenId;
bool isActive;
}

// @> The functions expect a uint32, forcing dangerous downcasting
function list(uint256 _tokenId, uint32 _price) external onlyWhitelisted { ... }
function updatePrice(uint256 _listingId, uint32 _newPrice) external onlySeller(_listingId) { ... }

Risk

Likelihood:

Medium. High-value NFTs or simple user choices can easily exceed the 4,294 USDC threshold. Frontends utilizing explicit casting to match the ABI will trigger the truncation silently.

Impact:

High. Instant and massive loss of funds for the seller. The NFT will be listed at a fraction of its intended value (e.g., a 5,000 USDC intended listing is truncated to ~705 USDC) and will be immediately sniped by MEV bots or fast buyers.

Proof of Concept

Add the following test to test/NFTDealersPoC.t.sol to prove the silent truncation.

Solidity
function test_PoC_SilentPriceTruncation() public {
vm.startPrank(attacker);
dealers.mintNft();

    uint256 intendedPrice = 5000e6; // 5000 USDC
    
    // Explicit casting to match the ABI signature bypasses Solidity 0.8 overflow checks
    dealers.list(1, uint32(intendedPrice)); 
    vm.stopPrank();

    (, uint32 actualPriceInContract, , , ) = dealers.s_listings(1); 
    
    // The contract stores 705032704 (705 USDC) instead of 5000000000 (5000 USDC)
    assertLt(actualPriceInContract, intendedPrice);
    assertEq(actualPriceInContract, 705032704); 
}

Recommended Mitigation

Refactor the Listing struct and the respective function parameters to use uint256 for all price-related variables. The gas saved by storage packing a uint32 is heavily outweighed by the critical risk of silent truncation and loss of funds.

Diff
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) {
  // ...
  }