NFT Dealers
First Flight #58
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Protocol Pays Double Collateral Due to Missing State Reset in collectUsdcFromSelling

velo

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

Lead Judging Commences

rubik0n Lead Judge 16 days ago
Submission Judgement Published
Validated
Assigned finding tags:

drain-protocol-risk

collateral is not reset to zero after collecting USDC from sold NFT. No accounting for collected USDC

Support

FAQs

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

Give us feedback!