Weather Witness
First Flight #40
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Impact: high
Likelihood: high
Invalid

[H-2] Premature Timestamp Update in performUpkeep() leads to Silent Failures and DoS Risk

agmguindo

Root + Impact

[H-2] Premature Timestamp Update in performUpkeep() leads to Silent Failures and DoS Risk

Description

The WeatherNft::performUpkeep() function updates the lastFulfilledAt timestamp before confirming that the Chainlink Functions request has successfully executed and updated the weather data.

This premature state change leads to a false positive in checkUpkeep(), which will return false until the heartbeat interval has passed, even if the previous update failed. As a result, no retry will be triggered during that interval, creating a silent denial of service for that NFT's weather update.

In addition, silent failures occur in both the fulfillMintRequest() and _fulfillWeatherUpdate() functions:

if (response.length == 0 || err.length > 0) {
return;
}

This early return prevents further execution when the Chainlink Functions request fails, but does not emit any event or log the error. As a result, we will have not updated NFTs and blocked retry of the performUpkeep till the time has passed.

Both contribute to a poor user experience and complicate debugging or monitoring of failed Chainlink executions.

​
function performUpkeep(bytes calldata performData) external override {
uint256 _tokenId = abi.decode(performData, (uint256));
uint256 upkeepId = s_weatherNftInfo[_tokenId].upkeepId;
​
@> s_weatherNftInfo[_tokenId].lastFulfilledAt = block.timestamp;
​
// make functions request
string memory pincode = s_weatherNftInfo[_tokenId].pincode;
string memory isoCode = s_weatherNftInfo[_tokenId].isoCode;
​
bytes32 _reqId = _sendFunctionsWeatherFetchRequest(pincode, isoCode);
//mapping para controlar updates
s_funcReqIdToTokenIdUpdate[_reqId] = _tokenId;
​
emit NftWeatherUpdateRequestSend(_tokenId, _reqId, upkeepId);
//* de aqui se ejecuta automaticamente fulfillRequest() que o actualiza en
//* en este caso el Nft a un nuevo Weather o crea un nuevo NFT
}
​

Risk

Likelihood:

  • Reason 1
    High Likelihood: This issue can occur at any time due to external factors like billing, gas limits, or function errors.

Impact:

  • Impact 1
    High Severity: This issue breaks the core functionality of the protocol — the automatic weather updates for NFTs. If a Chainlink Functions call fails, it is not retried, leaving NFTs outdated indefinitely. Users may believe their NFTs are up to date while in fact, they are not.

Proof of Concept

PoC

We have updated the lastFulfilledAt but the nft has failed to be updated. The time for the next retry has been restarted.

​
function test_audit_SilenceErrorWhenUpdatingTimeStampBeforeCompletation()
public
{
string memory pincode = "125001";
string memory isoCode = "IN";
bool registerKeeper = true;
uint256 heartbeat = 12 hours;
uint256 initLinkDeposit = 5e18;
uint256 tokenId = weatherNft.s_tokenCounter(); //1
​
vm.startPrank(user);
linkToken.approve(address(weatherNft), initLinkDeposit); //aprueba que se le envien esos tokesn de mi a SC weather
vm.recordLogs();
weatherNft.requestMintWeatherNFT{
value: weatherNft.s_currentMintPrice()
}(pincode, isoCode, registerKeeper, heartbeat, initLinkDeposit);
vm.stopPrank();
​
// saca de logs el reqId
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;
}
}
assert(reqId != bytes32(0));
​
(
uint256 reqHeartbeat,
address reqUser,
bool reqRegisterKeeper,
uint256 reqInitLinkDeposit,
string memory reqPincode,
string memory reqIsoCode
) = weatherNft.s_funcReqIdToUserMintReq(reqId);
assertEq(reqUser, user);
assertEq(reqHeartbeat, heartbeat);
assertEq(reqInitLinkDeposit, initLinkDeposit);
assertEq(reqIsoCode, isoCode);
assertEq(reqPincode, pincode);
assertEq(reqRegisterKeeper, registerKeeper);
​
// esta mockeando la respuesta del Functions, hardcode la respuesta
vm.prank(functionsRouter);
bytes memory weatherResponse = abi.encode(
WeatherNftStore.Weather.RAINY
);
​
weatherNft.handleOracleFulfillment(reqId, weatherResponse, "");
​
vm.prank(user);
weatherNft.fulfillMintRequest(reqId);
​
(
uint256 infoHeartbeat,
uint256 infoLastFulfilledAt,
uint256 infoUpkeepId,
string memory infoPincode,
string memory infoIsoCode
) = weatherNft.s_weatherNftInfo(tokenId);
assertEq(infoHeartbeat, heartbeat);
assertEq(infoIsoCode, isoCode);
assertEq(infoLastFulfilledAt, block.timestamp);
assertEq(infoPincode, pincode);
assert(infoUpkeepId > 0);
assertEq(
uint8(weatherNft.s_tokenIdToWeather(tokenId)),
uint8(WeatherNftStore.Weather.RAINY)
);
​
// getting token uri
string memory tokenURI = weatherNft.tokenURI(tokenId);
​
// automation check
bytes memory encodedTokenId = abi.encode(tokenId);
(bool weatherUpdateNeeded, ) = weatherNft.checkUpkeep(encodedTokenId);
assert(weatherUpdateNeeded == false);
​
// time travelling to reach heartbeat for weather update
vm.warp(block.timestamp + heartbeat);
​
(bool weatherUpdateNeeded2, bytes memory performData) = weatherNft
.checkUpkeep(encodedTokenId);
assert(weatherUpdateNeeded2 == true);
assertEq(performData, encodedTokenId);
​
uint256 expectedTimeStamp = block.timestamp;
vm.recordLogs();
weatherNft.performUpkeep(performData);
@> //! We have updated the lastFulfilledAt but the nft has failed to be updated.
//! The time for the next retry has been restarted
​
(, uint256 lastFulfilledAt, , , ) = weatherNft.s_weatherNftInfo(
tokenId
);
​
assertEq(lastFulfilledAt, expectedTimeStamp);
​
// fetching request id
Vm.Log[] memory entries = vm.getRecordedLogs();
uint256 tokenIdToUpdate;
bytes32 tokenIdUpdateReq;
for (uint256 i; i < entries.length; i++) {
if (
entries[i].topics[0] ==
keccak256(
"NftWeatherUpdateRequestSend(uint256,bytes32,uint256)"
)
) {
(tokenIdToUpdate, tokenIdUpdateReq) = abi.decode(
entries[i].data,
(uint256, bytes32)
);
break;
}
}
​
assert(tokenIdUpdateReq != bytes32(0));
assertEq(tokenIdToUpdate, tokenId);
assertEq(
weatherNft.s_funcReqIdToTokenIdUpdate(tokenIdUpdateReq),
tokenId
);
​
vm.prank(functionsRouter);
bytes memory newWeatherResponse = abi.encode(
WeatherNftStore.Weather.CLOUDY
);
weatherNft.handleOracleFulfillment(tokenIdUpdateReq, "", "Error");
​
string memory newTokenURI = weatherNft.tokenURI(tokenId);
assertNotEq(tokenURI, newTokenURI);
}

Recommended Mitigation

We recommend to delay the update of lastFulfilledAt until the weather update has been successfully processed in WeatherNft::_fulfillWeatherUpdate()

​
function performUpkeep(bytes calldata performData) external override {
uint256 _tokenId = abi.decode(performData, (uint256));
uint256 upkeepId = s_weatherNftInfo[_tokenId].upkeepId;
​
​
- s_weatherNftInfo[_tokenId].lastFulfilledAt = block.timestamp;
​
// make functions request
string memory pincode = s_weatherNftInfo[_tokenId].pincode;
string memory isoCode = s_weatherNftInfo[_tokenId].isoCode;
​
bytes32 _reqId = _sendFunctionsWeatherFetchRequest(pincode, isoCode);
//mapping para controlar updates
s_funcReqIdToTokenIdUpdate[_reqId] = _tokenId;
​
emit NftWeatherUpdateRequestSend(_tokenId, _reqId, upkeepId);
}

Handle errors explicitly in WeatherNft::_fulfillWeatherUpdate():

​
if (response.length == 0 || err.length > 0) {
+ emit UpdateRequestFailed(requestId, err); // for minting
return;
}
Updates

Appeal created

bube Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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