Weather Witness
First Flight #40
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Contract Locks Ether Without a Withdrawal Function

rubencrxz

Description

The WeatherNftSol::requestMintWeatherNFT function is marked payable and collects ETH from users as minting fees. However, the contract provides no mechanism for the owner (or any authorized party) to withdraw the accumulated ETH. As a result, all ETH sent to the contract remains permanently locked, preventing the project from accessing its minting revenue.

Vulnerability Details

Missing Withdrawal Mechanism: Although the contract accepts ETH via a payable function, there is no owner-only withdrawal function.

No Fallback/Receive Handler: The contract lacks a fallback() or receive() function that could facilitate ETH recovery or forwarding.

Impact

Lost Revenue: Minting fees accumulate indefinitely with no way to retrieve them.

Operational Risk: Inability to withdraw funds undermines project sustainability and damages user trust.

Tools Used

This issue was found by using Aderyn and Slither.

Recommended Mitigation

Add a secure, owner-only withdrawal function that adheres to the checks-effects-interactions pattern. It should:

Verify that the contract’s balance is positive.

(If applicable) Update any relevant state variables before transferring funds.

Transfer ETH using a low-level call and revert on failure.

Emit an event for auditability.

Implement the withdrawEther function in WeatherNft.sol and the corresponding EtherWithdrawn event in WeatherNftStore.sol file.

contract WeatherNftStore {
.
.
.
event WeatherNFTMintRequestSent(address user, string pincode, string isoCode, bytes32 reqId);
event WeatherNFTMinted(bytes32 reqId, address user, Weather weather);
event NftWeatherUpdateRequestSend(uint256 tokenId, bytes32 reqId, uint256 upkeepId);
event NftWeatherUpdated(uint256 tokenId, Weather weather);
+ event EtherWithdrawn(uint256 amount);
contract WeatherNft is WeatherNftStore, ERC721, FunctionsClient, ConfirmedOwner, AutomationCompatibleInterface {
.
.
.
/// @notice Withdraw all ETH from the contract to the owner
function withdrawEther() external onlyOwner {
uint256 balance = address(this).balance;
require(balance > 0, "No ETH to withdraw");
​
// Interactions last: transfer funds
(bool success, ) = payable(owner()).call{ value: balance }("");
require(success, "ETH withdrawal failed");
​
emit EtherWithdrawn(balance);
}
​

To ensure that only the contract’s owner can withdraw Ether, follow these steps:

  1. Declare and initialize the owner
    Define a private owner variable and set it to msg.sender in the constructor so that the deployer becomes the owner.

  2. Create the onlyOwner modifier
    Implement a modifier that checks require(msg.sender == owner, "Caller is not the owner"); and then runs the rest of the function.

  3. Protect withdrawEther
    Simply add onlyOwner to the withdrawEther function signature. This guarantees that any attempt to call it from a non-owner address will revert..

Updates

Appeal created

bube Lead Judge 7 days ago
Submission Judgement Published
Validated
Assigned finding tags:

Lack of `withdraw` function

The contract collects funds for minting a WeatherNFT, but there is no function that allows the owner to withdraw these funds.

Support

FAQs

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