Weather Witness

First Flight #40
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Submission Details
Impact: medium
Likelihood: medium
Invalid

Zero Heartbeat Interval Leads to Rapid LINK Drain for Automated Updates

Root + Impact

Description

When a user mints an NFT and opts for automated weather updates, they specify a _heartbeat interval. This interval determines how frequently Chainlink Automation should check and update the NFT's weather data. The contract does not validate the _heartbeat value provided by the user.
If a user provides _heartbeat = 0, the condition (block.timestamp >= s_weatherNftInfo[_tokenId].lastFulfilledAt + s_weatherNftInfo[_tokenId].heartbeat) in checkUpkeep simplifies to (block.timestamp >= s_weatherNftInfo[_tokenId].lastFulfilledAt). This condition will be true in the same block performUpkeep updates lastFulfilledAt, or in the very next block.
Consequently, checkUpkeep will continuously indicate that an update is needed. Each time performUpkeep is called, it initiates a new Chainlink Functions request and consumes LINK from the NFT's dedicated keeper subscription for the performUpkeep transaction itself. This rapid, continuous triggering will quickly deplete the _initLinkDeposit provided for the NFT's automation.

// Root cause in the codebase with @> marks to highlight the relevant section
// File: src/WeatherNft.sol
function requestMintWeatherNFT(
string memory _pincode,
string memory _isoCode,
bool _registerKeeper,
// @> uint256 _heartbeat, // User-provided, no validation (can be 0)
uint256 _initLinkDeposit
) external payable returns (bytes32 _reqId) {
// ...
s_funcReqIdToUserMintReq[_reqId] = UserMintRequest({
// ...
// @> heartbeat: _heartbeat, // Stored as provided
// ...
});
}
function fulfillMintRequest(bytes32 requestId) external {
// ... (assuming user fix and state cleanup fixes from prior reports)
UserMintRequest memory _userMintRequest = s_funcReqIdToUserMintReq[requestId];
require(_userMintRequest.user != address(0), "WeatherNft__InvalidRequest");
// ...
uint256 tokenId = s_tokenCounter;
// ...
s_weatherNftInfo[tokenId] = WeatherNftInfo({
// @> heartbeat: _userMintRequest.heartbeat, // Set in NFT info
lastFulfilledAt: block.timestamp,
upkeepId: upkeepId, // Assuming upkeepId is set correctly
pincode: _userMintRequest.pincode,
isoCode: _userMintRequest.isoCode
});
// ...
}
function checkUpkeep(
bytes calldata checkData
)
external
view
override
returns (bool upkeepNeeded, bytes memory performData)
{
uint256 _tokenId = abi.decode(checkData, (uint256));
if (_ownerOf(_tokenId) == address(0)) { // NFT must exist
upkeepNeeded = false;
}
else {
// @> upkeepNeeded = (block.timestamp >= s_weatherNftInfo[_tokenId].lastFulfilledAt + s_weatherNftInfo[_tokenId].heartbeat);
// If s_weatherNftInfo[_tokenId].heartbeat is 0, upkeepNeeded is true if block.timestamp >= lastFulfilledAt.
// This will be true almost immediately after lastFulfilledAt is updated.
if (upkeepNeeded) {
performData = checkData;
}
}
}
function performUpkeep(bytes calldata performData) external override {
uint256 _tokenId = abi.decode(performData, (uint256));
// ... (access checks for keeper or owner should be here, though Automation Compatible implies keeper calls)
// @> s_weatherNftInfo[_tokenId].lastFulfilledAt = block.timestamp; // Timestamp updated
string memory pincode = s_weatherNftInfo[_tokenId].pincode;
string memory isoCode = s_weatherNftInfo[_tokenId].isoCode;
// A new Chainlink Functions request is made, consuming LINK from the keeper subscription for this tx
// and potentially from the Functions subscription s_functionsConfig.subId.
bytes32 _reqId = _sendFunctionsWeatherFetchRequest(pincode, isoCode);
s_funcReqIdToTokenIdUpdate[_reqId] = _tokenId;
emit NftWeatherUpdateRequestSend(_tokenId, _reqId, s_weatherNftInfo[_tokenId].upkeepId);
}

Risk

Likelihood: Medium

  • A user might accidentally input 0 if a UI allows it, or a front-end might default to 0 if not handled.

  • A user might intentionally set it to 0 if they misunderstand the implications, leading to self-inflicted LINK loss for their automation.

Impact: Medium

  • Rapid Depletion of Automation LINK Funds: The _initLinkDeposit for the specific NFT's automation upkeep will be consumed very quickly due to frequent performUpkeep calls. This renders the automation feature short-lived and wastes the user's deposit.

  • Increased Load on Chainlink Services: Frequent, unnecessary calls to performUpkeep and subsequently to Chainlink Functions can put a small, but potentially cumulative, load on the Chainlink network.

  • Wasted Gas and Oracle Fees: Each cycle of performUpkeep and the Functions call costs gas (paid by keeper, funded by user's LINK deposit) and LINK for the Functions request itself (paid from the contract's Functions subscription, which might be shared or owner-funded).

Proof of Concept

  1. Alice mints an NFT and calls requestMintWeatherNFT with _pincode = "12345", _isoCode = "US", _registerKeeper = true, _heartbeat = 0, and _initLinkDeposit = 5 * 10**18 (5 LINK).

  2. The NFT is minted (assuming fulfillMintRequest is called and completes). s_weatherNftInfo[tokenId].heartbeat is set to 0. s_weatherNftInfo[tokenId].lastFulfilledAt is set to T1 (current block timestamp).

  3. A Chainlink Keeper node calls checkUpkeep for this NFT in block T1 or T1 + few seconds (e.g., next block).

    • The condition block.timestamp >= T1 + 0 is true.

    • checkUpkeep returns upkeepNeeded = true.

  4. The Keeper node calls performUpkeep. This transaction costs LINK from Alice's upkeep subscription.

    • s_weatherNftInfo[tokenId].lastFulfilledAt is updated to T2 (current block timestamp during performUpkeep).

    • A Chainlink Functions request is sent.

  5. Another (or the same) Keeper node calls checkUpkeep shortly after, in block T2 or T2 + few seconds.

    • The condition block.timestamp >= T2 + 0 is true.

    • checkUpkeep returns upkeepNeeded = true.

  6. This cycle repeats rapidly, each performUpkeep call consuming LINK, until Alice's 5 LINK deposit for automation is exhausted. The shared Chainlink Functions subscription (s_functionsConfig.subId) will also be charged for each weather API request.

Recommended Mitigation

Add a require statement in requestMintWeatherNFT to ensure that _heartbeat is above a reasonable minimum value (e.g., a few minutes or hours, depending on the desired operational parameters and Chainlink node capabilities). This prevents a zero or extremely small heartbeat interval that would lead to excessive updates.

// File: src/WeatherNft.sol
// Define a constant for minimum heartbeat
+ uint256 private constant MIN_HEARTBEAT_INTERVAL = 5 minutes; // Example: 5 minutes. Adjust based on project needs.
function requestMintWeatherNFT(
string memory _pincode,
string memory _isoCode,
bool _registerKeeper,
uint256 _heartbeat,
uint256 _initLinkDeposit
) external payable returns (bytes32 _reqId) {
require(
msg.value == s_currentMintPrice, // Price check
WeatherNft__InvalidAmountSent()
);
// Assuming s_currentMintPrice increment is moved to fulfillMintRequest (per other recommendation)
if (_registerKeeper) {
// Assuming _initLinkDeposit <= type(uint96).max check (per other recommendation)
require(_initLinkDeposit <= type(uint96).max, "WeatherNft__LinkDepositExceedsMax");
+ require(_heartbeat >= MIN_HEARTBEAT_INTERVAL, "WeatherNft__HeartbeatTooShort");
IERC20(s_link).safeTransferFrom(
msg.sender,
address(this),
_initLinkDeposit
);
}
// If _registerKeeper is false, _heartbeat is stored but not used by this contract's automation.
// Consider if validation is needed even if _registerKeeper is false, or if heartbeat
// should only be settable/validated if _registerKeeper is true.
// For this fix, validation is tied to keeper registration as that's where it has direct impact.
_reqId = _sendFunctionsWeatherFetchRequest(_pincode, _isoCode);
emit WeatherNFTMintRequestSent(msg.sender, _pincode, _isoCode, _reqId);
s_funcReqIdToUserMintReq[_reqId] = UserMintRequest({
user: msg.sender,
pincode: _pincode,
isoCode: _isoCode,
registerKeeper: _registerKeeper,
heartbeat: _heartbeat, // Now validated if _registerKeeper is true
initLinkDeposit: _initLinkDeposit
});
}
Updates

Appeal created

bube Lead Judge 5 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

[Invalid] Lack of input validation in `requestMintWeatherNFT`

This is informational. It is user's responsibility to provide correct input arguments. If the user provides incorrect arguments, it will lead to incorrect results, lost funds or failed transaction.

Support

FAQs

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