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

`cancelListing` Returns Minting Collateral While Seller Retains NFT

Author Revealed upon completion

cancelListing Returns Minting Collateral While Seller Retains NFT

Description

When a seller mints an NFT, they pay an amount as collateral. This collateral is intended to be held by the protocol until the NFT's lifecycle ends. However, cancelListing incorrectly transfers the full minting collateral back to the seller and zeros out the mapping, even though the seller still retains ownership of the NFT after cancellation.

function cancelListing(uint256 _listingId) external {
Listing memory listing = s_listings[_listingId];
if (!listing.isActive) revert ListingNotActive(_listingId);
require(listing.seller == msg.sender, "Only seller can cancel listing");
​
s_listings[_listingId].isActive = false;
activeListingsCounter--;
​
// @> collateral returned to seller even though seller still holds the NFT
usdc.safeTransfer(listing.seller, collateralForMinting[listing.tokenId]);
collateralForMinting[listing.tokenId] = 0;
​
emit NFT_Dealers_ListingCanceled(_listingId);
}

Risk

Likelihood: High
Any whitelisted seller can exploit this by listing and immediatelly cancelling to reclaim their collateral deposit at no cost.

Impact: High
The minting collateral can be reclaimed while still holding the NFT.

Proof of Concept

function test_CancelListing_ReturnsCollateralToSeller() public {
// Initialize test
vm.startBroadcast(owner);
nftDealers.whitelistWallet(seller);
nftDealers.revealCollection();
vm.stopBroadcast();
​
// Assert state after initialization
assertEq(nftDealers.whitelistedUsers(seller), true);
assertEq(nftDealers.isCollectionRevealed(), true);
​
// Mint
vm.startBroadcast(seller);
usdc.approve(address(nftDealers), USDC_COLLATERAL);
nftDealers.mintNft();
vm.stopBroadcast();
​
// Assert state after mint
uint256 tokenId = 1;
assertEq(nftDealers.ownerOf(tokenId), seller);
assertEq(nftDealers.collateralForMinting(tokenId), USDC_COLLATERAL);
assertEq(usdc.balanceOf(address(nftDealers)), INITIAL_USER_BALANCE + USDC_COLLATERAL);
assertEq(usdc.balanceOf(seller), INITIAL_USER_BALANCE - USDC_COLLATERAL);
​
// List NFT
uint32 sellingPrice = 10e6;
vm.prank(seller);
nftDealers.list(tokenId, sellingPrice);
​
// Assert state after list
(address _seller, uint32 _price, address _nft, uint256 _tokenId, bool _isActive) =
nftDealers.s_listings(tokenId);
assertEq(_seller, seller);
assertEq(_price, sellingPrice);
assertEq(_nft, address(nftDealers));
assertEq(_tokenId, tokenId);
assertEq(_isActive, true);
assertEq(nftDealers.ownerOf(tokenId), seller);
​
// Cancel listing
vm.prank(seller);
nftDealers.cancelListing(tokenId);
​
// Assert state after cancelling listing
(_seller, _price, _nft, _tokenId, _isActive) = nftDealers.s_listings(tokenId);
assertEq(_seller, seller);
assertEq(_price, sellingPrice);
assertEq(_nft, address(nftDealers));
assertEq(_tokenId, tokenId);
assertEq(_isActive, false);
assertEq(nftDealers.ownerOf(tokenId), seller);
// Collateral returned back to seller
assertEq(usdc.balanceOf(address(nftDealers)), INITIAL_USER_BALANCE);
assertEq(usdc.balanceOf(seller), INITIAL_USER_BALANCE);
}

Recommended Mitigation

Do not return the collateral in cancelListing. The collateral should only be released after a confirmed sale.

function cancelListing(uint256 _listingId) external {
Listing memory listing = s_listings[_listingId];
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);
}

Support

FAQs

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

Give us feedback!