Root + Impact

Description

• The buy and cancelListing functions violate the Checks-Effects-Interactions (CEI) pattern.

• They execute external calls (_safeTransfer for NFTs and safeTransfer for USDC) before updating the internal state variables (isActive = false or collateralForMinting = 0).

Risk

Likelihood:

• Every time buy and cancelListing are called

Impact:

• In cancelListing, a reentrant call can be used to trigger the usdc.safeTransfer multiple times before the collateralForMinting

• Corruption of the activeListingsCounter state and potential for double-claiming collateral.

Proof Of Concept:

Initial Call: An attacker contract calls buy(ID).

• The Interaction: NFTDealers executes _safeTransfer, which triggers the attacker's onERC721Received hook before the listing is marked as inactive.

• The Re-entry: Inside the hook, the attacker calls buy(ID) again.

• Logic Failure: The second call passes the isActive check because the first call is still "paused" on the stack and hasn't reached the line to flip the bool.

• Corruption: The activeListingsCounter is decremented a second time (causing a revert/underflow), proving the contract's internal accounting is compromised.

Recommended Mitigation

• In buy: Move s_listings[_listingId].isActive = false and activeListingsCounter-- above the _safeTransfer call.

• In cancelListing: Move collateralForMinting[listing.tokenId] = 0 above the usdc.safeTransfer call.