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

Incorrect Token Transfer Logic Allows Seller to Drain Contract Funds

Author Revealed upon completion

Root + Impact

Description

  • Normal Behavior:
    After an NFT is sold, the seller should be able to collect their sale proceeds minus fees, and retrieve their locked collateral. The contract should correctly transfer USDC from its own balance to the seller and accumulate fees separately.

Issue:
In collectUsdcFromSelling(), the contract incorrectly attempts to transfer fees using safeTransfer(address(this), fees), which sends tokens from the contract to itself instead of deducting or accounting properly. This results in incorrect accounting and allows sellers to repeatedly claim funds, draining the contract.

function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
Listing memory listing = s_listings[_listingId];
​
uint256 fees = _calculateFees(listing.price);
uint256 amountToSeller = listing.price - fees;
uint256 collateralToReturn = collateralForMinting[listing.tokenId];
​
totalFeesCollected += fees;
amountToSeller += collateralToReturn;
​
usdc.safeTransfer(address(this), fees); //@> incorrect transfer (no effect, funds stay in contract)
usdc.safeTransfer(msg.sender, amountToSeller); //@> seller receives full payout
​
}

Risk

Likelihood:High

  • The vulnerable function is part of the normal sale flow and is expected to be called by every seller after a sale.

  • No restrictions prevent repeated calls or misuse once a listing becomes inactive.

Impact: High

  • Sellers can receive more funds than intended due to incorrect fee handling.

  • Contract accounting becomes inconsistent, allowing gradual or full draining of USDC held in the contract.

Proof of Concept

Explanation

After a successful NFT sale, the seller calls collectUsdcFromSelling() to claim their funds.
However, the contract incorrectly transfers the fee to itself instead of properly deducting it from the payout.

Because of this:

  • The fee is not actually separated or deducted

  • The seller receives more funds than intended

  • The contract accounting becomes inconsistent

This allows sellers to repeatedly extract value from the contract.

// Step 1: Seller lists NFT
nftDealers.list(tokenId, 1000e6);
​
// Step 2: Buyer purchases NFT
nftDealers.buy(tokenId);
​
// Step 3: Seller collects funds
nftDealers.collectUsdcFromSelling(tokenId);
​
// Issue:
// fees are "transferred" to the contract itself (no real deduction)
// seller still receives full payout + collateral

Recommended Mitigation

Explanation

The contract should not transfer fees to itself, since the funds are already held within the contract.

Instead, it should:

  • Remove the redundant self-transfer

  • Only transfer the correct net amount to the seller

  • Ensure fees are properly accounted for

  • Prevent multiple claims by resetting state

function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
Listing memory listing = s_listings[_listingId];
require(!listing.isActive, "Listing must be inactive");
​
uint256 fees = _calculateFees(listing.price);
uint256 collateralToReturn = collateralForMinting[listing.tokenId];
​
uint256 amountToSeller = listing.price - fees + collateralToReturn;
​
totalFeesCollected += fees;
​
// REMOVE THIS (incorrect)
// usdc.safeTransfer(address(this), fees);
​
// Correct payout
usdc.safeTransfer(msg.sender, amountToSeller);
​
// Prevent double claim
collateralForMinting[listing.tokenId] = 0;
}

Support

FAQs

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

Give us feedback!