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

Collateral not cleared in the function collectUsdcFromSelling allows repeated withdrawal of locked funds

Author Revealed upon completion

Description

The function `collectUsdcFromSelling()` allows a seller to withdraw both the sale proceeds and the collateral locked during minting.

However, the contract does not reset the collateral after it is withdrawn.

Specifically, the mapping `collateralForMinting[tokenId]` is never set to zero after being transferred to the seller.

This allows the same collateral amount to be repeatedly included in future withdrawals.

Root

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;
​
@> // Missing: collateralForMinting[listing.tokenId] = 0;
​
usdc.safeTransfer(msg.sender, amountToSeller);
}

Impact

A malicious seller can repeatedly call collectUsdcFromSelling() and receive the same collateral amount multiple times.

This results in:

  • Repeated extraction of locked collateral

  • Loss of funds from the contract

  • Incorrect accounting of collateral

When sufficient balance exists in the contract, this can lead to significant fund loss.

function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId)
{
Listing memory listing = s_listings[_listingId];
require(!listing.isActive, "Listing must be inactive to collect USDC");
​
@> // Missing state tracking to prevent multiple withdrawals
@> // No check to ensure funds are claimed only once
​
uint256 fees = _calculateFees(listing.price);

Risk

Likelihood:

  • Reason 1 - The function is callable by the seller without any restriction on the number of times it can be executed.

  • Reason 2 - No state is updated to prevent reuse of collateral, making exploitation trivial.

Impact:

  • Impact 1 - A malicious seller can repeated extraction of locked collateral.

  • Impact 2 - Loss of funds from the contract.

Proof of Concept

Explanation

This process can be repeated to extract collateral multiple times.

1. Seller mints an NFT (collateral is locked).
2. Seller lists and sells the NFT.
3. Seller calls `collectUsdcFromSelling()` and receives:
- Sale proceeds
- Collateral
​
4. Seller calls `collectUsdcFromSelling()` again.
5. The same collateral is returned again because it was never cleared.

Recommended Mitigation

Explanation:

The fix ensures that collateral is only used once by resetting `collateralForMinting[tokenId]` to zero before transferring funds.

This follows the Checks-Effects-Interactions pattern and prevents repeated extraction of the same collateral in subsequent calls.

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];
​
// Prevent multiple withdrawals of collateral
+ require(collateralToReturn > 0, "Collateral already claimed");
​
// Effects: clear collateral before external interaction
+ collateralForMinting[listing.tokenId] = 0;
​
totalFeesCollected += fees;
amountToSeller += collateralToReturn;
​
- usdc.safeTransfer(msg.sender, amountToSeller);
+ usdc.safeTransfer(msg.sender, amountToSeller);
}
​

Support

FAQs

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

Give us feedback!