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

The contract has a payable function `WeatherNft::requestMintWeatherNFT` that accepts ETH but there is no withdrawal function in the contract so any ETH sent to the contract is permanently locked

one618xyz

The contract has a payable function WeatherNft::requestMintWeatherNFT that accepts ETH but there is no withdrawal function in the contract so any ETH sent to the contract is permanently locked

Description

  • There should be withdrawal function for the contract owner to withdraw collected ETH

  • The lack of withdrawl function directly lead to loss of funds (the ETH is still in the contract)

// as this is a payable function
function requestMintWeatherNFT(
string memory _pincode,
string memory _isoCode,
bool _registerKeeper,
uint256 _heartbeat,
uint256 _initLinkDeposit
@> ) external payable returns (bytes32 _reqId)

Risk

Likelihood:

  • The contract will accumulate ETH as more users mint NFTs, with the price increasing by s_stepIncreasePerMint for each mint

  • The contract will receive ETH payments every time a user mints a Weather NFT through the requestMintWeatherNFT function

Impact:

  • All ETH sent to the contract becomes permanently locked as there is no withdrawal mechanism

  • The contract owner cannot access the collected minting fees, potentially leading to significant financial loss

  • Users' payments are effectively trapped in the contract with no way to recover them

Proof of Concept

function test_LockedEther() public {
// Setup initial balances
address user1 = makeAddr("user1");
address user2 = makeAddr("user2");
vm.deal(user1, 1 ether);
vm.deal(user2, 1 ether);
// Get initial contract balance
uint256 initialContractBalance = address(weatherNft).balance;
// First user mints NFT
vm.startPrank(user1);
uint256 mintPrice1 = weatherNft.s_currentMintPrice();
weatherNft.requestMintWeatherNFT{value: mintPrice1}(
"12345",
"US",
false,
1 hours,
0
);
vm.stopPrank();
// Second user mints NFT (price should be higher)
vm.startPrank(user2);
uint256 mintPrice2 = weatherNft.s_currentMintPrice();
weatherNft.requestMintWeatherNFT{value: mintPrice2}(
"67890",
"US",
false,
1 hours,
0
);
vm.stopPrank();
// Check contract balance has increased
uint256 finalContractBalance = address(weatherNft).balance;
assertEq(finalContractBalance, initialContractBalance + mintPrice1 + mintPrice2, "Contract balance should increase by mint prices");
// Try to withdraw as owner (should fail as there's no withdraw function)
vm.startPrank(weatherNft.owner());
(bool success, ) = address(weatherNft).call(abi.encodeWithSignature("withdraw()"));
assertFalse(success, "Withdraw should fail as function doesn't exist");
vm.stopPrank();
// Verify ETH is locked
assertEq(address(weatherNft).balance, finalContractBalance, "Contract balance should remain unchanged");
// Verify user balances decreased
assertEq(user1.balance, 1 ether - mintPrice1, "User1 balance should decrease by mint price");
assertEq(user2.balance, 1 ether - mintPrice2, "User2 balance should decrease by mint price");
}
}

Recommended Mitigation

// Add a withdrawal function to WeatherNft.sol
+ function withdraw() external onlyOwner {
+ (bool success, ) = payable(owner()).call{value: address(this).balance}("");
+ require(success, "Withdrawal failed");
+ }
​
// Or if the ETH is meant for specific purposes, add a function to transfer to those addresses
+ function transferToKeeperRegistry() external onlyOwner {
+ uint256 amount = address(this).balance;
+ (bool success, ) = payable(s_keeperRegistry).call{value: amount}("");
+ require(success, "Transfer to keeper registry failed");
+ }
Updates

Appeal created

bube Lead Judge 5 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.