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

Unnecessary `payable` on `buy()` and `mintNft()` permanently locks ETH

Author Revealed upon completion

Description

  • mintNft() and buy() are both marked payable, which allows callers to attach ETH to these transactions. All payments in the protocol are made exclusively in USDC via transferFrom.

  • Neither function reads msg.value or processes the attached ETH in any way. The contract has no receive(), no fallback(), and no ETH withdrawal function, so any ETH sent with these calls is permanently locked in the contract with no recovery path.

@> function mintNft() external payable onlyWhenRevealed onlyWhitelisted {
// msg.value is never checked or used
...
require(usdc.transferFrom(msg.sender, address(this), lockAmount), "USDC transfer failed");
...
}
​
@> function buy(uint256 _listingId) external payable {
// msg.value is never checked or used
...
bool success = usdc.transferFrom(msg.sender, address(this), listing.price);
...
}
​
// no receive(), no fallback(), no withdrawETH()

Risk

Likelihood:

  • Any user who mistakenly or deliberately sends ETH along with a mintNft or buy call loses those funds immediately.

  • Wallets and scripts that batch ETH and ERC20 operations can trigger this silently.

Impact:

  • ETH sent to these functions is permanently locked — there is no owner function or emergency withdrawal to recover it.

  • Users suffer direct financial loss with no recourse.

Proof of Concept

Paste this function inside NFTDealersTest in test/NFTDealersTest.t.sol and run:
forge test --match-test testPoC_PayableBuyLocksEth -vvvv

function testPoC_PayableBuyLocksEth() public {
vm.prank(owner);
nftDealers.revealCollection();
vm.prank(owner);
nftDealers.whitelistWallet(userWithCash);
​
// userWithCash mints and lists (setUp already gave them 20e6 USDC)
vm.startPrank(userWithCash);
usdc.approve(address(nftDealers), 20e6);
nftDealers.mintNft();
nftDealers.list(1, uint32(100e6));
vm.stopPrank();
​
address buyer = makeAddr("buyer");
usdc.mint(buyer, 100e6);
vm.deal(buyer, 1 ether);
​
vm.startPrank(buyer);
usdc.approve(address(nftDealers), 100e6);
// Buyer mistakenly (or via a buggy UI) sends 1 ETH alongside the USDC payment
nftDealers.buy{value: 1 ether}(1);
vm.stopPrank();
​
// Transaction succeeded but 1 ETH is now locked — no withdraw function exists
assertEq(address(nftDealers).balance, 1 ether);
assertEq(buyer.balance, 0);
}

Recommended Mitigation

- 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!