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

`payable` modifier on `mintNft()` and `buy()` permanently locks any ETH sent to the contract

Author Revealed upon completion

Description

mintNft() and buy() handle payments exclusively in USDC. These functions should not accept ETH since the contract has no use for it and no mechanism to return or withdraw it.

Both functions are marked payable, so they silently accept ETH alongside the USDC transfer. The contract has no withdraw, rescue, or receive function. Any ETH sent is permanently trapped.

@> function mintNft() external payable onlyWhenRevealed onlyWhitelisted {
if (msg.sender == address(0)) revert InvalidAddress();
require(tokenIdCounter < MAX_SUPPLY, "Max supply reached");
require(msg.sender != owner, "Owner can't mint NFTs");
​
require(usdc.transferFrom(msg.sender, address(this), lockAmount), "USDC transfer failed");
// ETH accepted but never used — no withdraw function exists
}
​
@> function buy(uint256 _listingId) external payable {
// Same issue — payable accepts ETH, contract only uses USDC
}

Risk

Likelihood:

  • Every time a user sends ETH with a mintNft() or buy() call, the ETH is accepted and locked. Users who mistake the payment token will trigger this.

Impact:

  • All ETH sent to the contract is permanently lost. There is no admin function, no rescue mechanism, and no fallback to return it. The loss is proportional to the amount of ETH accidentally sent.

Proof of Concept

A user calls mintNft() with 0.5 ETH attached, expecting it might be the payment method. The function succeeds — USDC collateral is transferred, the NFT is minted. But the 0.5 ETH is now trapped in the contract. There is no withdrawEth() function. Neither the user nor the owner can recover it.

function test_poc_ethLockedInMintNft() public {
assertEq(address(nftDealers).balance, 0);
assertEq(user.balance, 1 ether);
​
// User calls mintNft with ETH attached — function is payable, accepts it
vm.startPrank(user);
usdc.approve(address(nftDealers), LOCK_AMOUNT);
nftDealers.mintNft{value: 0.5 ether}();
vm.stopPrank();
​
// Mint succeeded, but 0.5 ETH is permanently trapped
assertEq(nftDealers.ownerOf(1), user);
assertEq(address(nftDealers).balance, 0.5 ether);
​
// No function exists to recover the ETH
vm.prank(protocolOwner);
(bool success,) = address(nftDealers).call(abi.encodeWithSignature("withdrawEth()"));
assertFalse(success);
}

Recommended Mitigation

Remove the payable modifier from both functions. The contract only uses USDC for payments, so there is no reason to accept ETH. Removing payable makes the EVM revert any call that sends ETH, preventing accidental loss.

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

Support

FAQs

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

Give us feedback!