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

Permanent Locking of LINK Tokens for Automation in Multiple Scenarios

albert

Root + Impact

Description

The contract handles LINK tokens for Chainlink Automation (Keepers). Users deposit LINK when minting an NFT with _registerKeeper = true. This LINK is intended to fund the upkeep for that NFT. However, there are scenarios where these LINK tokens can become permanently locked:

  1. Failed Keeper Registration: If IAutomationRegistrarInterface(s_keeperRegistrar).registerUpkeep() in fulfillMintRequest fails for any reason (e.g., temporary registrar issue, misconfiguration, insufficient gas provided by the user calling fulfillMintRequest), the transaction may revert. The _initLinkDeposit amount, already transferred to the contract in requestMintWeatherNFT, remains in the contract with no mechanism for the user to reclaim it.

  2. No De-registration/Withdrawal Function: Once an upkeep is successfully registered, its initial funding (_initLinkDeposit) is transferred to the Chainlink Automation system. The contract (address(this)) is set as the adminAddress for the upkeep. There are no functions in WeatherNft.sol to allow the NFT owner (or contract owner) to:

    • Cancel an active upkeep (e.g., if the user no longer wants automated updates).

    • Withdraw remaining LINK funds from a canceled or completed upkeep.

    • Withdraw LINK funds if an NFT is burned or transferred, and automation is no longer desired/possible for the original owner.
      This means any LINK committed to an upkeep or stuck in the contract due to registration failure is effectively lost to the user.

// Root cause in the codebase with @> marks to highlight the relevant section
// File: src/WeatherNft.sol
​
function requestMintWeatherNFT(
// ...
uint256 _initLinkDeposit
) external payable returns (bytes32 _reqId) {
// ...
if (_registerKeeper) {
// @> IERC20(s_link).safeTransferFrom( // LINK transferred to this contract
msg.sender,
address(this),
_initLinkDeposit
);
}
// ...
s_funcReqIdToUserMintReq[_reqId] = UserMintRequest({
// ...
// @> initLinkDeposit: _initLinkDeposit // Stored amount
});
}
​
function fulfillMintRequest(bytes32 requestId) external {
// ... (assume user fix from previous vulnerability)
UserMintRequest memory _userMintRequest = s_funcReqIdToUserMintReq[requestId];
// ...
uint256 upkeepId;
if (_userMintRequest.registerKeeper) {
LinkTokenInterface(s_link).approve(s_keeperRegistrar, _userMintRequest.initLinkDeposit);
​
IAutomationRegistrarInterface.RegistrationParams
memory _keeperParams = IAutomationRegistrarInterface
.RegistrationParams({
// ...
// @> adminAddress: address(this), // Contract is admin of upkeep
// ...
// @> amount: uint96(_userMintRequest.initLinkDeposit) // LINK to be used by registrar
});
​
// @> upkeepId = IAutomationRegistrarInterface(s_keeperRegistrar).registerUpkeep(_keeperParams);
// If registerUpkeep reverts, the transaction reverts. _initLinkDeposit remains in contract.
// No function exists to withdraw this LINK from the contract by the user.
}
// ...
s_weatherNftInfo[tokenId] = WeatherNftInfo({
// ...
// @> upkeepId: upkeepId, // Upkeep ID stored
// ...
});
// ...
}
​
// Missing functions:
// - function to allow NFT owner or contract owner to cancel upkeep associated with a tokenId (e.g., cancelUpkeep(tokenId))
// This function would call IAutomationRegistryMaster(s_keeperRegistry).cancelUpkeep(s_weatherNftInfo[tokenId].upkeepId).
// - function to allow NFT owner or contract owner to withdraw funds from a canceled upkeep
// (e.g., withdrawFromUpkeep(tokenId, recipient, amount))
// This function would call IAutomationRegistryMaster(s_keeperRegistry).withdrawFunds(s_weatherNftInfo[tokenId].upkeepId, recipient).
// - function for user to reclaim _initLinkDeposit if registerUpkeep failed and LINK is stuck in contract.

Risk

Likelihood: High

  • Keeper registration can fail due to external reasons or gas limits.

  • Users will likely want to stop automation or recover funds if they no longer use the NFT or the service, which is a standard feature for such systems. The absence of this feature guarantees LINK will be locked eventually for many upkeeps.

Impact: Medium

  • Permanent Loss of LINK: Users permanently lose their _initLinkDeposit if registration fails and it's stuck in the contract, or they lose any unspent LINK in the Automation system once an upkeep is registered and later becomes unwanted/unused.

  • Reduced Capital Efficiency: Locked LINK cannot be repurposed by users.

  • Contract Bloat (Minor): LINK tokens may accumulate in the contract address if many registrations fail.

Proof of Concept

Scenario 1: Failed Registration

  1. Alice calls requestMintWeatherNFT with _registerKeeper = true and deposits 5 LINK. The 5 LINK is transferred to the WeatherNft contract.

  2. Alice calls fulfillMintRequest. During the execution, IAutomationRegistrarInterface(s_keeperRegistrar).registerUpkeep() fails (e.g., the registrar is temporarily paused by Chainlink admins, or the specific call runs out of gas).

  3. The fulfillMintRequest transaction reverts.

  4. Alice's 5 LINK remains in the WeatherNft contract. There is no function Alice can call to get her 5 LINK back.

Scenario 2: No De-registration

  1. Bob mints an NFT with automation, and 5 LINK is successfully used to fund the upkeep via registerUpkeep. s_weatherNftInfo[tokenId].upkeepId is set.

  2. After some time, Bob sells the NFT or no longer wants automated updates. The upkeep has 3 LINK remaining in its balance within the Chainlink Automation system.

  3. Bob (or the new owner) cannot cancel the upkeep or withdraw the remaining 3 LINK because WeatherNft.sol does not provide any functions to interact with the s_keeperRegistry for upkeep management (cancel, withdraw funds). The 3 LINK remains locked in the Automation system, associated with that upkeep ID.

Recommended Mitigation

Implement functions to manage the lifecycle of automations and associated funds:

  1. Refund LINK on Registration Failure: If registerUpkeep can fail, and the fulfillMintRequest transaction doesn't revert fully (or if it does, provide a way to claim LINK stuck in contract), ensure users can reclaim their _initLinkDeposit that's held by the contract. This is linked to the general "funds loss" vulnerability.

  2. Cancel Upkeep: Add a function, callable by the NFT owner (or contract owner, with appropriate checks), to cancel an upkeep using IAutomationRegistryMaster(s_keeperRegistry).cancelUpkeep(upkeepId). The upkeepId is stored in s_weatherNftInfo[tokenId].upkeepId.

  3. Withdraw Funds from Upkeep: Add a function, callable by the NFT owner (or contract owner), to withdraw remaining funds from a (typically canceled) upkeep using IAutomationRegistryMaster(s_keeperRegistry).withdrawFunds(upkeepId, recipientAddress). The funds would be sent to the specified recipientAddress.

  4. Ownership/Admin Control: Decide who should have the authority to cancel and withdraw (NFT owner vs. contract owner). If NFT owner, ensure proper ownership checks.

// File: src/WeatherNft.sol
// Add IAutomationRegistryMaster if not already implicitly available or import its specific interface
// import {IAutomationRegistryMaster} from "@chainlink/contracts/src/v0.8/automation/interfaces/v2_2/IAutomationRegistryMaster.sol"; // Already imported
​
// ... (inside WeatherNft contract)
​
// --- Add these new functions ---
​
// Function to allow NFT owner to cancel their automation
function cancelNftAutomation(uint256 tokenId) external {
_requireOwned(tokenId); // Ensure caller owns the NFT
WeatherNftInfo storage weatherInfo = s_weatherNftInfo[tokenId];
require(weatherInfo.upkeepId != 0, "WeatherNft__NoAutomationRegistered");
​
IAutomationRegistryMaster(s_keeperRegistry).cancelUpkeep(weatherInfo.upkeepId);
// Optionally, clear or mark upkeepId as cancelled
// weatherInfo.upkeepId = 0; // Or a status field
emit NftAutomationCancelled(tokenId, weatherInfo.upkeepId);
}
​
// Function to allow NFT owner to withdraw funds from their (presumably cancelled) automation upkeep
// Note: Chainlink Automation upkeeps might send funds back to the admin (this contract) upon cancellation.
// Check Chainlink documentation for the exact behavior of fund withdrawal post-cancellation.
// If funds are returned to this contract, then a mechanism to withdraw from this contract is needed.
// If IAutomationRegistryMaster.withdrawFunds can send to an arbitrary address, NFT owner can specify themselves.
function withdrawNftAutomationFunds(uint256 tokenId, address to) external {
_requireOwned(tokenId); // Ensure caller owns the NFT
WeatherNftInfo storage weatherInfo = s_weatherNftInfo[tokenId];
require(weatherInfo.upkeepId != 0, "WeatherNft__NoAutomationRegisteredOrAlreadyCancelled");
// Add check: Ensure upkeep is actually cancellable or funds withdrawable,
// e.g., by checking its state via s_keeperRegistry.getUpkeep(weatherInfo.upkeepId)
​
IAutomationRegistryMaster(s_keeperRegistry).withdrawFunds(weatherInfo.upkeepId, to);
// Clear upkeepId after successful withdrawal/cancellation
// weatherInfo.upkeepId = 0;
emit NftAutomationFundsWithdrawn(tokenId, weatherInfo.upkeepId, to);
}
​
// Function for users to claim LINK if it was deposited but keeper registration failed
// This is more complex and tied to the "User Loses Funds" issue.
// A robust system would track `initLinkDeposit` for `requestId` if registration fails
// and allow `_userMintRequest.user` to claim it back.
// For example (highly simplified, needs more robust state management):
// mapping(address => uint256) public s_stuckLinkDeposits; // user => amount
// In fulfillMintRequest, if registerUpkeep fails AND _initLinkDeposit > 0:
// s_stuckLinkDeposits[_userMintRequest.user] += _userMintRequest.initLinkDeposit;
//
// function claimStuckLinkDeposit() external {
// uint256 amount = s_stuckLinkDeposits[msg.sender];
// require(amount > 0, "No stuck LINK to claim");
// s_stuckLinkDeposits[msg.sender] = 0;
// LinkTokenInterface(s_link).transfer(msg.sender, amount);
// }
​
// Event for new functions
event NftAutomationCancelled(uint256 tokenId, uint256 upkeepId);
event NftAutomationFundsWithdrawn(uint256 tokenId, uint256 upkeepId, address to);
​

Note: The exact implementation of withdrawNftAutomationFunds depends on how Chainlink Automation handles fund withdrawals (e.g., if funds are returned to the admin contract first, or can be sent directly to a user-specified address). The IAutomationRegistryMaster interface includes withdrawFunds(uint256 id, address to), which suggests direct withdrawal to a specified address is possible.

Updates

Appeal created

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