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

mintNft and buy accept ETH but have no withdrawal mechanism, permanently trapping sent ETH

Author Revealed upon completion

Root + Impact

mintNft() and buy() are declared payable but neither function uses msg.value, and the contract provides no ETH withdrawal or recovery mechanism. Any ETH attached to those calls is permanently locked in the contract.

Vulnerability Details

// @> payable but msg.value is never used or refunded
function mintNft() external payable onlyWhenRevealed onlyWhitelisted {
// ... no msg.value check, no refund
}
​
// @> payable but settlement is USDC-only via transferFrom
function buy(uint256 _listingId) external payable {
// ...
bool success = usdc.transferFrom(msg.sender, address(this), listing.price);
// msg.value is ignored entirely
}

The contract inherits from ERC721 but does not implement receive() or fallback(), and there is no withdrawETH owner function. ETH sent to either entry point cannot be recovered by users or the owner.

Risk

Likelihood:

  • Easy user mistake, especially from generic frontends, wallet UIs that auto-attach ETH, or scripted integrations

  • Occurs whenever a user mistakenly sends ETH alongside a USDC transaction

Impact:

  • ETH sent to mintNft() or buy() is permanently trapped

  • No recovery path exists for affected users or the protocol owner

Proof of Concept

function testEthSentToMintIsPermanentlyTrapped() public {
vm.startPrank(owner);
nftDealers.revealCollection();
nftDealers.whitelistWallet(alice);
vm.stopPrank();
​
vm.deal(alice, 1 ether);
vm.startPrank(alice);
usdc.approve(address(nftDealers), type(uint256).max);
// Alice accidentally sends 1 ETH with the mint call
nftDealers.mintNft{value: 1 ether}();
vm.stopPrank();
​
// ETH is now locked — no withdrawal function exists
assertEq(address(nftDealers).balance, 1 ether);
assertEq(alice.balance, 0);
}

Recommended Mitigation

Remove the payable modifier from both functions since ETH payment is not part of the protocol design:

-function mintNft() external payable onlyWhenRevealed onlyWhitelisted {
+function mintNft() external onlyWhenRevealed onlyWhitelisted {
​
-function buy(uint256 _listingId) external payable {
+function buy(uint256 _listingId) external {

Alternatively, add an explicit revert if msg.value > 0 to prevent accidental ETH transfers, or add an owner-only ETH recovery function if ETH acceptance is intentional.

Support

FAQs

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

Give us feedback!