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

Lack of Token Recovery Mechanism for Stuck Assets

0x68

Description

  • The WeatherNft contract handles both ETH and LINK tokens as part of its core functionality for minting NFTs and registering upkeeps.

  • The contract lacks any mechanism to recover tokens that may get stuck in the contract, including tokens sent by mistake or LINK tokens remaining after operations are complete.

contract WeatherNft is WeatherNftStore, ERC721, FunctionsClient, ConfirmedOwner, AutomationCompatibleInterface {
// @> No function exists to recover tokens that are accidentally sent to the contract
// @> or remain after operations are complete
}

Risk

Likelihood: Medium

  • Users may mistakenly send ERC20 tokens to the contract address directly

  • LINK token balances might remain in the contract after upkeep cancellations or contract migrations

  • The contract does not handle refunds if a Functions request fails

Impact: Medium

  • Any tokens accidentally sent to the contract will be permanently locked

  • In case of contract upgrades or migrations, remaining LINK balances cannot be transferred to a new contract

  • Contract owner cannot recover valuable assets even in emergency situations

Proof of Concept

The contract accepts LINK tokens for Chainlink Automation registration:

if (_registerKeeper) {
IERC20(s_link).safeTransferFrom(
msg.sender,
address(this),
_initLinkDeposit
);
}

However, there's no way to withdraw these tokens if:

  1. The registration fails but tokens were already transferred

  2. The upkeep is canceled later but LINK tokens remain in the contract

  3. Other ERC20 tokens are accidentally sent to the contract

Recommended Mitigation

Implement a token recovery function that allows the owner to withdraw any ERC20 tokens:

+ /**
+ * @notice Allows the contract owner to recover any ERC20 tokens sent to the contract by mistake
+ * @param tokenAddress The address of the token to recover
+ * @param to The address to send the recovered tokens to
+ * @param amount The amount of tokens to recover
+ */
+ function recoverERC20(address tokenAddress, address to, uint256 amount) external onlyOwner {
+ IERC20(tokenAddress).safeTransfer(to, amount);
+
+ emit TokenRecovered(tokenAddress, to, amount);
+ }
+
+ event TokenRecovered(address indexed token, address indexed to, uint256 amount);

Additionally, consider implementing a similar function for recovering native ETH that might be accidentally sent to the contract:

+ /**
+ * @notice Allows the contract owner to recover ETH sent to the contract by mistake
+ * @param to The address to send the recovered ETH to
+ * @param amount The amount of ETH to recover
+ */
+ function recoverETH(address payable to, uint256 amount) external onlyOwner {
+ (bool success, ) = to.call{value: amount}("");
+ require(success, "ETH recovery failed");
+
+ emit ETHRecovered(to, amount);
+ }
+
+ event ETHRecovered(address indexed to, uint256 amount);
Updates

Appeal created

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

Locked LINK tokens

Support

FAQs

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