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

SafeTransfer(address(this), fees) is a self-transfer no-op

Author Revealed upon completion

Root + Impact

Description

  • In collectUsdcFromSelling(), fees are "transferred" from the contract to
    itself. The USDC balance is unchanged — no fees are actually segregated. When
    withdrawFees() later sends totalFeesCollected to the owner, it draws from
    other users' funds (collateral and sale proceeds).

function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
uint256 fees = _calculateFees(listing.price);
totalFeesCollected += fees;
@> usdc.safeTransfer(address(this), fees); // contract sends to itself
usdc.safeTransfer(msg.sender, amountToSeller);
}

Risk

Likelihood:

  • Triggered on every collect call. The self-transfer is
    inherent to the code path.

Impact:

  • totalFeesCollected grows but the contract's balance doesn't
    increase by fees. withdrawFees() depletes funds that belong to sellers and
    minters.

Proof of Concept

The self-transfer moves zero net USDC. The accounting variable
totalFeesCollected inflates. When the owner withdraws, funds come from other
users' collateral and pending proceeds.

function test_selfTransferNoOp() public {
uint256 contractBal = usdc.balanceOf(address(nft));
vm.prank(alice);
nft.collectUsdcFromSelling(1);
// totalFeesCollected increased, but contract balance decreased
// by amountToSeller only — fees stayed in contract as part of
// the general pool, not segregated
}

Recommended Mitigation

Remove the self-transfer. Fees remain in the contract naturally; only send the
seller's amount out.

function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_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);
usdc.safeTransfer(msg.sender, amountToSeller);
}

Support

FAQs

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

Give us feedback!