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

Unused Storage Variable `s_keeperRegistry`

elyas6126

Root + Impact

The storage variable s_keeperRegistry (declared in WeatherNftStore and initialized in WeatherNft) that is never used in any function, wasting gas during deployment and potentially indicating incomplete upkeep management functionality, which could lead to permanent loss of users' LINK tokens if upkeeps need to be canceled.

Description

The Chainlink Automation system requires both a Registrar (to register upkeeps) and a Registry (to manage/cancel them) for full lifecycle support. However, the WeatherNft contract initializes the Registry address (s_keeperRegistry) without ever using it, while only leveraging the Registrar (s_keeperRegistrar) to create upkeeps. This imbalance allows upkeep creation but prevents cancellation or management, risking permanently locked LINK tokens if upkeeps require termination.

(Key issues: Unused Registry, asymmetric functionality, financial risk.)

contract WeatherNft is WeatherNftStore, ERC721, FunctionsClient, ConfirmedOwner, AutomationCompatibleInterface {
.
.
.
constructor ( ,
,
,
,
,
,
,
@> address _keeperRegistey,
,
) {
,
,
,
,
,
,
,
@> s_keeperRegistry = _keeperRegistry
,
}

Risk

Likelihood:

  • Reason 1 The issue occurs whenever an NFT owner needs to cancel an upkeep (e.g., when they no longer want weather updates)

  • Reason 2 The issue occurs when the contract needs upgrading or replacing, as the existing upkeeps cannot be properly terminated

Impact:

  • Impact 1 Users' LINK tokens remain locked in upkeeps with no cancellation mechanism

  • Impact 2 Upkeeps continue to execute and consume LINK tokens even if no longer desired

Proof of Concept

The WeatherNft contract stores the Chainlink Automation Registry address (s_keeperRegistry) but never uses it.

This means there's no function to cancel automated weather updates (upkeeps). As a result, LINK tokens deposited by users for these upkeeps become permanently locked in the Chainlink system, with no way for the user or contract owner to recover them through the WeatherNft contract.

function test_LockLinkTokensInUpkeep() public {
// STEP 1: Alice mints an NFT with automation enabled
string memory pincode = "125001";
string memory isoCode = "IN";
bool registerKeeper = true;
uint256 heartbeat = 12 hours;
uint256 initLinkDeposit = 5e18; // 5 LINK tokens
uint256 tokenId = weatherNft.s_tokenCounter();
// Alice approves LINK tokens for the contract and requests minting
vm.startPrank(user);
linkToken.approve(address(weatherNft), initLinkDeposit);
vm.recordLogs();
weatherNft.requestMintWeatherNFT{value: weatherNft.s_currentMintPrice()}(
pincode, isoCode, registerKeeper, heartbeat, initLinkDeposit
);
vm.stopPrank();
// Process the minting similar to WeatherNftForkTest
Vm.Log[] memory logs = vm.getRecordedLogs();
bytes32 reqId;
for (uint256 i; i < logs.length; i++) {
if (logs[i].topics[0] == keccak256("WeatherNFTMintRequestSent(address,string,string,bytes32)")) {
(,,, reqId) = abi.decode(logs[i].data, (address, string, string, bytes32));
break;
}
}
// Oracle fulfills the request
vm.prank(functionsRouter);
bytes memory weatherResponse = abi.encode(WeatherNftStore.Weather.RAINY);
weatherNft.handleOracleFulfillment(reqId, weatherResponse, "");
// Alice completes the mint
vm.prank(user);
weatherNft.fulfillMintRequest(reqId);
// STEP 2: Verify upkeep was created successfully
(,, uint256 upkeepId,,) = weatherNft.s_weatherNftInfo(tokenId);
assertTrue(upkeepId > 0, "Upkeep should be created");
console.log("Upkeep ID:", upkeepId);
console.log("LINK tokens deposited:", initLinkDeposit / 1e18, "LINK");
// STEP 3: Demonstrate the issue
// Document available functions to verify absence of cancellation ability
console.log("\nAvailable functions in WeatherNft contract:");
// List would normally include all public/external functions
console.log("- requestMintWeatherNFT");
console.log("- fulfillMintRequest");
console.log("- performUpkeep");
console.log("- checkUpkeep");
console.log("- tokenURI");
console.log("- NO cancelUpkeep function exists!");
// Check if the Registry address is stored but unused
address registryAddress = weatherNft.s_keeperRegistry();
console.log("\nRegistry address stored but never used:", registryAddress);
// Try to interact with Registry as the NFT owner (user)
vm.startPrank(user);
console.log("\nAttempt to cancel upkeep as user:");
console.log("Result: Cannot cancel - no function available in WeatherNft contract");
vm.stopPrank();
// Try to interact with Registry as contract owner
address contractOwner = makeAddr("owner");
vm.startPrank(contractOwner);
console.log("\nAttempt to cancel upkeep as contract owner:");
console.log("Result: Cannot cancel - no function available in WeatherNft contract");
vm.stopPrank();
// VERIFICATION: Show what's missing
console.log("\nCode that should exist but doesn't:");
console.log("function cancelUpkeep(uint256 upkeepId) external onlyOwner {");
console.log(" IRegistryInterface(s_keeperRegistry).cancelUpkeep(upkeepId);");
console.log("}");
console.log("\nCONCLUSION: User's 5 LINK tokens are permanently locked");
}

Recommended Mitigation

  1. Add two event in the WeatherNftStore contract:

+ event UpkeepCancelled(uint256 indexed tokenId, uint256 indexed upkeepId, address indexed cancelledBy);
+ event UpkeepFundsWithdrawn(uint256 indexed upkeepId, address indexed reciever, address indexed withdrawnBy);

  1. Implement proper upkeep cancellation functionality using the s_keeperRegistry address in the WeatherNft contract:

+ functoin cancelUpkeep(uint256 tokenId) external {
+ require(msg.sender == ownerOf(tokenId) || msg.sender == owner(), "WeatherNft: Not authorized to cancel upkeep");
+ WeatherNftInfo storage nftInfo = s_weatherNftInfo[tokenId],
+ require(nftInfo.upkeepId != 0, "No active upkeep for this token id");
+ uint256 upkeepIdTocancel = nftInfo.upkeepId;
+ nftInfo.upkeepId = 0;
+ IAutomationRegistryMaster(s_keeperRegistry).cancelUpkeep(upkeepIdToCancel);
+ emit UpkeepCancelled(tokenId, upkeepId, msg.sender);
+}
​
+ function withdrawFundFromCancelledUpkeep(uint256 cancelledUpkeepId, address payable to) external onlyOwner {
+ IAutomationRegistryMaster(s_keeperRegistry).withdrawFunds(tokenId, to);
+ emit UpkeepFundsWithdrawn(cancelledUpkeepId, to, msg.sender);
+}
Updates

Appeal created

bube Lead Judge about 1 month ago
Submission Judgement Published
Validated
Assigned finding tags:

Locked LINK tokens

Support

FAQs

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