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

Useless Self-Transfer

Author Revealed upon completion

Root + Impact

Description

  • The protocol collects fees from each sale and the owner should be able to withdraw accumulated fees later.

  • The collectUsdcFromSelling function transfers the fee amount to address(this), which is the contract itself, accomplishing nothing and wasting gas.

solidity

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); // contract sends to itself?
usdc.safeTransfer(msg.sender, amountToSeller);
}

Risk

Likelihood:

  • This happens every single time a seller collects their USDC after a sale

  • The fees are already sitting in the contract from when the buyer paid, so moving them to the same address does nothing

Impact:

  • Wastes gas on every collection

  • Makes the code confusing and harder to audit

  • The fee accounting doesn't work properly because the transfer is redundant

Proof of Concept

solidity

// Bob buys NFT for 1000 USDC
// Contract receives 1000 USDC and holds it
// Contract balance: 1020 USDC (1000 + 20 collateral)
​
// Alice calls collectUsdcFromSelling
fees = 50 USDC
amountToSeller = 950 USDC + 20 collateral = 970 USDC
​
// Executes
usdc.safeTransfer(address(this), 50); // contract balance stays same
usdc.safeTransfer(alice, 970); // sends to seller
​
// Final state
// Contract balance: 50 USDC (unchanged by self-transfer)
// Just wasted gas moving funds to the same address

Recommended Mitigation

diff

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);
}

Support

FAQs

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

Give us feedback!