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

Contract locks Ether without a withdraw function

Author Revealed upon completion

Contract locks Ether without a withdraw function

Description:

It appears that the contract includes a payable function to accept Ether but lacks a corresponding function to withdraw it, which leads to the Ether being locked in the contract. To resolve this issue, please implement a public or external function that allows for the withdrawal of Ether from the contract.

Risk

Likelihood: Medium

Even the protocol doesn't require ETH, both NFTDealers::mintNft and NFTDealers::buy are payable functions. This means that users can accidentally send ETH with the transaction when interacting with the contract.

In practice this can happen due to:

  • User mistakes when sending transactions from wallets like MetaMask.

  • Frontend bugs where ETH value is unintentionally attached.

  • Scripted interactions / bots that attach ETH by default.

  • Developers or integrators misunderstanding the payment logic (since the protocol uses USDC).

Because Ethereum transactions allow sending ETH to any payable function, the contract cannot prevent users from attaching ETH unless payable is removed.

Impact: High

Any ETH sent to mintNft or buy is permanently locked in the contract. Since the protocol exclusively uses USDC for all payments, payable serves no purpose. Users who accidentally send ETH alongside a transaction lose it forever — the owner has no mechanism to recover it

Proof Of Concept

Add this test to NFTDealersTest.t.sol

function test_ETHLockedInContract() public revealed whitelisted(){
uint256 lockAmount = nftDealers.lockAmount();
​
usdc.mint(userWithCash, lockAmount);
vm.startPrank(userWithCash);
vm.deal(userWithCash, 1 ether);
usdc.approve(address(nftDealers), lockAmount);
nftDealers.mintNft{value: 1 ether}();
vm.stopPrank();
​
assertEq(address(nftDealers).balance, 1 ether);
​
vm.prank(owner);
vm.expectRevert();
(bool success, ) = address(nftDealers).call{value: 1 ether}(
abi.encodeWithSignature("withdrawFees()")
);
​
assertEq(address(nftDealers).balance, 1 ether);
console.log(address(nftDealers).balance);
}
Logs:
1000000000000000000

Recommended Mitigation

Since ETH plays no role in the protocol, simply remove payable from both functions:

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

This prevents ETH from ever entering the contract. Or, if you plan to receive ETH, implement a withdraw function that withdraws ETH to the owner.

Support

FAQs

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

Give us feedback!