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

`NFTDealers::mintNft()` and `NFTDealers::buy()` are marked `payable` with no ETH handling causing permanent ETH loss

Author Revealed upon completion

NFTDealers::mintNft() and NFTDealers::buy() are marked payable with no ETH handling causing permanent ETH loss

Description

`NFTDealers::mintNft()` and `NFTDealers::buy()` are marked `payable` despite the protocol exclusively using USDC for all payment flows. The `payable` keyword explicitly allows ETH to be sent alongside these calls, unlike non-payable functions which automatically revert on ETH receipt at the compiler level. However, the contract contains no `receive()` `fallback()`, no `withdraw()` function, and no `msg.value` validation, meaning any ETH sent to these functions becomes permanently locked with no recovery path.

function mintNft() external payable onlyWhenRevealed onlyWhitelisted { /* @> payable */ }
function buy(uint256 _listingId) external payable { /* @> payable */ }

Risk

Users who accidentally send ETH alongside their transaction will permanently lose those funds with no recovery path. The contract has no withdraw function for ETH and no way for the owner to recover stuck ETH.

Likelihood: MEDIUM

  • User can accidentally send ETH to thoose two function

Impact: MEDIUM

  • No protocol rule is violated but user's ETH will be sent to the contract

Proof of Concept

user can accidentally send ether to the contract, this case just only accident but the user's ETH cant be withdrawn, so permanent loss to the user's ETH

vm.deal(userWithCash, 2 ether);
uint256 userInitialBalance = address(userWithCash).balance;
​
vm.prank(userWithCash);
nftDealers.mintNft{value: 1 ether}();
// user's eth will sent to the contract
​
uint256 userAfterBalance = address(usesrWithCash).balance;
​
assertLt(userAfterBalance, 1 ether); // user spend gas also

Recommended Mitigation

Remove payable modifier from both functions since ETH payments are not intended:

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

Support

FAQs

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

Give us feedback!