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

`mintNft` and `buy` are unnecessarily marked `payable`, trapping any ETH sent

Author Revealed upon completion

Links

  • src/NFTDealers.sol:118 — mintNft() declared payable

  • src/NFTDealers.sol:145 — buy() declared payable

Vulnerability Details

Both mintNft and buy are marked payable despite only handling USDC (an ERC20 token) via transferFrom. The payable keyword is only needed for functions that accept native ETH via msg.value. Since these functions never use msg.value and the protocol operates entirely in USDC, the modifier serves no purpose.

// NFTDealers.sol:118
function mintNft() external payable onlyWhenRevealed onlyWhitelisted {
​
// NFTDealers.sol:145
function buy(uint256 _listingId) external payable {

If a user accidentally sends ETH with either call, the ETH is accepted by the contract. There is no withdraw or sweep function for ETH anywhere in the contract — only withdrawFees() which transfers USDC. Any ETH sent is permanently locked.

Impact

ETH accidentally sent to mintNft() or buy() is permanently stuck in the contract with no recovery mechanism. Removing payable would cause Solidity to automatically reject any ETH sent, protecting users from mistakes at zero cost.

Recommended Mitigation

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 {

Support

FAQs

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

Give us feedback!