NFT Dealers
First Flight #58
Beginner FriendlyFoundry
100 EXP
View all submissions
Previous Next
Submission Details
Impact: high
Likelihood: high

Cancel lisitng sending lock amount to whitelisted user (seller), allowing whitelisted user to mint entire MAX_SUPPLY with minimal capital

Author Revealed upon completion

Cancel lisitng sending lock amount to whitelisted user (seller), allowing whitelisted user to mint entire MAX_SUPPLY with minimal capital

Description

NFTDealers::cancelListing() incorrectly returns the minting collateral (lockAmount) back to the seller upon listing cancellation. The collateral was originally designed to be locked for the lifetime of the NFT, not the lifetime of the listing. This allows any whitelisted user to repeatedly mint and cancel listings to recycle the same USDC collateral indefinitely.

function cancelListing(uint256 _listingId) external {
Listing memory listing = s_listings[_listingId]; //! @> same as line 154
if (!listing.isActive) revert ListingNotActive(_listingId);
require(listing.seller == msg.sender, "Only seller can cancel listing");
​
s_listings[_listingId].isActive = false;
activeListingsCounter--;
​
usdc.safeTransfer(listing.seller, collateralForMinting[listing.tokenId]); // @>
collateralForMinting[listing.tokenId] = 0;
​
emit NFT_Dealers_ListingCanceled(_listingId);
}

Risk

An whitelisted user with only lockAmount (20 USDC) can mint the entire MAX_SUPPLY of 1000 NFT by repeating the pattern: mintNft() → list() → cancelListing(). This completely allowing 1 whitelisted user to hold all the MAX_SUPPLY NFT

Proof of Concept

  1. Attacker is whitelisted with only 20 USDC

  2. Call mintNft() — pays 20 USDC collateral, receives tokenId #1

  3. Call list(1, price) — listing created

  4. Call cancelListing(1) — receives 20 USDC collateral back

  5. Repeat steps 2-4 until tokenId #1000 (MAX_SUPPLY)

  6. Attacker now owns entire collection, spending only 20 USDC total

copy this code and run the test

​
function test_bloatingMaxSupplyJust20USDC() public whitelisted richWhitelisted revealed {
// userWithCash exploit to mint all MAX_SUPPLY on her own
for (uint256 i; i < nftDealers.MAX_SUPPLY(); ++i) {
vm.startPrank(userWithCash);
usdc.approve(address(nftDealers), type(uint256).max);
nftDealers.mintNft();
​
uint256 tokenId = i + 1; // internal calculation nftDealers tokenId
​
nftDealers.list(tokenId, uint32(nftDealers.MIN_PRICE()));
nftDealers.cancelListing(tokenId);
vm.stopPrank();
​
assertEq(nftDealers.ownerOf(tokenId), userWithCash);
}
​
// now userWithEvenMoreCash turn to try mint NFT
vm.startPrank(userWithEvenMoreCash);
usdc.approve(address(nftDealers), type(uint256).max);
vm.expectRevert();
nftDealers.mintNft();
}
​

Recommended Mitigation

Collateral should be locked for the lifetime of the NFT, not the listing. Remove collateral return from cancelListing() and instead implement a burnNft() function that returns collateral only when the NFT is destroyed:

function cancelListing(uint256 _listingId) external {
// ...
s_listings[_listingId].isActive = false;
activeListingsCounter--;
- usdc.safeTransfer(listing.seller, collateralForMinting[listing.tokenId]);
- collateralForMinting[listing.tokenId] = 0;
​
emit NFT_Dealers_ListingCanceled(_listingId);
}
​
+ function burnNft(uint256 _tokenId) external {
+ require(ownerOf(_tokenId) == msg.sender, "Not owner of NFT");
+ uint256 collateral = collateralForMinting[_tokenId];
+ collateralForMinting[_tokenId] = 0;
+ _burn(_tokenId);
+ usdc.safeTransfer(msg.sender, collateral);
+ }

Support

FAQs

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

Give us feedback!