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

[M-1] Users can get their locked collateral back after minting an NFT, which is breaking the feature

Author Revealed upon completion

Users can get their locked collateral back after minting an NFT, which is breaking the feature

Description

The lockAmount immutable variable is intended to be a USDC amount locked by users when they mint an NFT. This amount is locked in the contract and tracked with the collateralForMinting(tokenId => lockAmount) mapping. There are 2 ways for a user to get their locked amount back:

  1. By calling cancelListing, he will receive the locked amount of USDC and the collateralForMinting mapping will be set to zero for this unlisted token.

  2. By calling collectUsdcFromSelling, he will receive the locked amount of USDC, but the collateralForMinting mapping will not be updated for the listing's token.

In this finding, we will only talk about the first way to get the tokens back. This issue makes the locked tokens feature completely useless because any user can get the locked tokens back easily.

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);
}

This can look like a low-impact vulnerability because the default lockAmount is 20 USDC, but on other applications, it can be much higher for specific purposes.

Risk

Likelihood:

  • This will happen whenever a seller wants to get back his locked USDC amount

Impact:

  • The USDC lock-on-mint feature is not working

Proof of Concept

Actors:

  • Seller: A user minting an NFT and getting his locked USDC back by canceling his listing

Proof of Code:

function testGetCollateralBack() public {
// Seller has to be whitelisted and the collection revealed
vm.startPrank(owner);
nftDealers.whitelistWallet(seller);
nftDealers.revealCollection();
vm.stopPrank();
​
// The seller locks 20 USDC as collateral to mint an NFT
vm.assertEq(usdc.balanceOf(seller), 20e6); // Seller has 20 USDC before minting
uint256 tokenId = 1;
uint32 tokenPrice = 1000e6;
vm.startPrank(seller);
usdc.approve(address(nftDealers), 20e6);
nftDealers.mintNft();
​
// The seller locked 20 USDC to mint the NFT but he can get them back by calling list and cancelListing
vm.assertEq(usdc.balanceOf(seller), 0); // Seller has 0 USDC because he locked them as collateral for minting
nftDealers.list(tokenId, uint32(tokenPrice));
nftDealers.cancelListing(tokenId);
vm.stopPrank();
​
// Seller got back the 20 USDC collateral and still owns an unlisted NFT that can be listed again
vm.assertEq(usdc.balanceOf(seller), 20e6);
}

Recommended Mitigation

Don't refund the collateral to the seller on listing cancellation. If you want to let users get their locked USDC back if they didn't manage to sell an NFT, it should be done through a custom burn function.

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!