NFT Dealers

Description

The expected behavior of cancelListing() following CEI is that all state changes complete before any external call (the USDC transfer). The function correctly sets isActive = false before transferring, but then zeros collateralForMinting[listing.tokenId] only after the safeTransfer returns.

With the current USDC-only configuration this ordering is not directly exploitable — standard ERC20 transfer does not invoke any callbacks on the recipient. However, the protocol's README states it is compatible with "Any EVM" and the architecture does not enforce that the token remains USDC forever. If the payment token were ever changed to one that supports recipient callbacks (such as ERC777, which calls tokensReceived on the recipient), the seller could re-enter collectUsdcFromSelling() during the callback while collateralForMinting still holds its original value, claiming the collateral a second time.

This also applies to cross-function re-entry with collectUsdcFromSelling() more broadly: during the callback window, collateralForMinting[listing.tokenId] is non-zero and a canceled listing appears as an inactive listing, satisfying both guards in that function.

Risk

Likelihood:

• Not exploitable with the current USDC deployment

• Risk surfaces if: (a) the contract is redeployed with a different token, or (b) a future upgrade makes the token configurable

Impact:

• With a callback-capable token: the seller could double-claim their collateral through the cancelListing → callback → collectUsdcFromSelling re-entry path

• No present impact with USDC

Proof of Concept

No PoC test is provided because the vulnerability is not exploitable in the current deployment. The code path can be traced manually:

1. Seller calls cancelListing(_listingId)

2. isActive is set to false

3. usdc.safeTransfer(listing.seller, collateralForMinting[listing.tokenId]) fires

4. [callback-capable token only] token calls tokensReceived on the seller contract

5. Seller contract calls collectUsdcFromSelling(_listingId):

   • require(!listing.isActive) passes — isActive is already false

   • collateralForMinting[listing.tokenId] is still non-zero — returns collateral a second time

6. Outer cancelListing call resumes and sets collateralForMinting = 0 (too late)

Recommended Mitigation

Move collateralForMinting[listing.tokenId] = 0 to before the transfer. Capture the value in a local variable first so the correct amount is still sent.

Why this works: at the point safeTransfer fires, collateralForMinting is already zero. Any re-entrant call to collectUsdcFromSelling would return zero collateral and send only listing.price - fees — but since the listing was canceled (not sold), listing.price represents a price that was never paid, so the transfer would fail on insufficient balance. The double-claim path is fully closed.