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

Missing Input Validation in `requestMintWeatherNFT` breaks `fulfillMintRequest` resulting in loss of funds

blackgrease

Summary

The WeatherNft:requestMintWeatherNFT does not enforce the minimum amount needed for a successful registration on a Chainlink Node, meaning any amounts lower than the minimum will cause the fulfilMintRequest to revert when called and a user loosing funds.

Description

The WeatherNFT:requestMintWeatherNFT is missing input validation on the _initLinkDeposit amount to make sure it is equal to or above the minimum Link required to successfully register on a Chainlink Node and request a dynamic NFT.

This missing input validation, is not apparent immediately as the function requestMintWeatherNFT executes successfully. However, when WeatherNFT:fulfillMintRequest is later called, it will fail, specifically when the Chainlink Function registerUpkeep is invoked.

This will result in the caller to loosing the initial funds sent in requestMintWeatherNFT due to there being no way to amend the value of _initLinkDeposit in the user struct, UserMintRequest.

As of testing in the current environment, the minimum value for _initLinkDeposit is 1e17; any lower value will cause the fufillMintRequest to fail. This is a cross-function issue as the missing input validation in requestMintWeatherNFT affects fufillMintRequest.

Affected Areas

  1. The WeatherNFT:requestMintWeatherNFT

​
//@audit-high: no input validation on _initLinkDeposit
function requestMintWeatherNFT(
string memory _pincode,
string memory _isoCode,
bool _registerKeeper,
uint256 _heartbeat,
uint256 _initLinkDeposit
) external payable returns (bytes32 _reqId) {
require(msg.value == s_currentMintPrice, WeatherNft__InvalidAmountSent());
s_currentMintPrice += s_stepIncreasePerMint;
​
if (_registerKeeper) {
IERC20(s_link).safeTransferFrom(msg.sender, address(this), _initLinkDeposit);
}
​
_reqId = _sendFunctionsWeatherFetchRequest(_pincode, _isoCode); //@audit-info: interacting with Chainlink
​
emit WeatherNFTMintRequestSent(msg.sender, _pincode, _isoCode, _reqId);
​
s_funcReqIdToUserMintReq[_reqId] = UserMintRequest({
user: msg.sender,
pincode: _pincode,
isoCode: _isoCode,
registerKeeper: _registerKeeper,
heartbeat: _heartbeat,
initLinkDeposit: _initLinkDeposit
});
}
​

  1. The WeatherNFT:fulfillMintRequest

​
function fulfillMintRequest(bytes32 requestId) external {
bytes memory response = s_funcReqIdToMintFunctionReqResponse[requestId].response;
bytes memory err = s_funcReqIdToMintFunctionReqResponse[requestId].err;
​
require(response.length > 0 || err.length > 0, WeatherNft__Unauthorized());
​
if (response.length == 0 || err.length > 0) {
return;
}
​
UserMintRequest memory _userMintRequest = s_funcReqIdToUserMintReq[requestId];
uint8 weather = abi.decode(response, (uint8));
uint256 tokenId = s_tokenCounter;
s_tokenCounter++;
​
emit WeatherNFTMinted(requestId, msg.sender, Weather(weather));
​
_mint(msg.sender, tokenId);
s_tokenIdToWeather[tokenId] = Weather(weather);
​
uint256 upkeepId;
if (_userMintRequest.registerKeeper) { //@audit-high: This will fail due to the low Link value
// Register chainlink keeper to pull weather data in order to automate weather nft
LinkTokenInterface(s_link).approve(s_keeperRegistrar, _userMintRequest.initLinkDeposit);
​
IAutomationRegistrarInterface.RegistrationParams memory _keeperParams = IAutomationRegistrarInterface
.RegistrationParams({
name: string.concat("Weather NFT Keeper: ", Strings.toString(tokenId)),
encryptedEmail: "",
upkeepContract: address(this),
gasLimit: s_upkeepGaslimit,
adminAddress: address(this),
triggerType: 0,
checkData: abi.encode(tokenId),
triggerConfig: "",
offchainConfig: "",
amount: uint96(_userMintRequest.initLinkDeposit)
});
​
upkeepId = IAutomationRegistrarInterface(s_keeperRegistrar).registerUpkeep(_keeperParams);
}
​
s_weatherNftInfo[tokenId] = WeatherNftInfo({
heartbeat: _userMintRequest.heartbeat,
lastFulfilledAt: block.timestamp,
upkeepId: upkeepId,
pincode: _userMintRequest.pincode,
isoCode: _userMintRequest.isoCode
});
}
​

Impact

The severity of the missing input validation is High due to

  1. High Impact: there is a loss of funds attributed to the payable function requestMintWeatherNFT; once the _initLinkDeposit is set in the struct UserMintRequest it cannot be changed. This will result in a user having to resubmit a request for a Weather NFT at greater cost (as the price would have been increased - either by contract or another user).

  2. High Likelihood: there are no additional requirements for the vulnerability to be triggered; only a value that is less than Chainlinks minimum funding requirement.

Tools Used

  • Manual Review

  • Foundry for PoC

Proof of Concept

To prove the validity of the issue, I have created a runnable PoC that proves the cross-function issue.

Description

  1. A user calls WeatherNFT:requestMintWeatherNFT with registerKeeper as true and the value for _initLinkDeposit as 1e16.

  2. The function executes successfully and the user is returned a RequestID.

  3. The user then calls WeatherNFT:fulfillMintRequest with the returned RequestID

  4. The interaction with a Chainlink Node will revert as the provided Link deposit is not enough to successful fund the node.

Code

Run with: forge test --mt testCrossFunctionBugFromMissingInputValidation --fork-url $AVAX_FUJI_RPC_URL --via-ir

  • A revert is expected, therefore the test passes.

​
function testCrossFunctionBugFromMissingInputValidation() public {
uint256 currentMintPrice = weatherNft.s_currentMintPrice();
​
string memory pincode = "125001";
string memory isoCode = "IN";
bool registerKeeper = true;
uint256 heartbeat = 12 hours;
uint256 initLinkDeposit = 1e16; //This will cause fulfillMintRequest to fail. The Minimum link to register is 1e17
​
vm.startPrank(user);
linkToken.approve(address(weatherNft), initLinkDeposit);
vm.recordLogs();
bytes32 requestId = weatherNft.requestMintWeatherNFT{value: 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));
​
vm.prank(functionsRouter);
bytes memory weatherResponse = abi.encode(WeatherNftStore.Weather.RAINY);
weatherNft.handleOracleFulfillment(reqId, weatherResponse, "");
​
vm.prank(user);
vm.expectRevert();
weatherNft.fulfillMintRequest(reqId); //this will revert
}

Mitigation

In order to mitigate this issue, the _initLinkDesposit should be validated to make sure the first subscription passes.

Variable Declaration in WeatherNftStore

.....
address public s_keeperRegistrar;
uint32 public s_upkeepGaslimit;
+ uint256 public s_minimum_link_Deposit = 1e17; //can either be set directly or via constructor
​

Code Changes in WeatherNft

​
require(msg.value == s_currentMintPrice, WeatherNft__InvalidAmountSent());
+ require(_initLinkDeposit >= s_minimum_link_Deposit, "Not Enough Link for Initial Deposit");
s_currentMintPrice += s_stepIncreasePerMint;

Bonus: Flexibility

  • To account for any changes in Chainlink minimum requirement, a management function can also be added to alter the value of the minimum Link deposit required.

​
+ function updateMinimumLinkDeposit(uint256 _newMinimumLinkDeposit) external onlyOwner{
s_minimum_link_Deposit = _newMinimumLinkDeposit;
}
Updates

Appeal created

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

[Invalid] The LINK deposit is not checked

This is informational/invalid. If the LINK deposit is not enough, the function `registerUpkeep` will revert and it is responsibility of the user to provide the correct amount of `_initLinkDeposit`, if the user wants automated weather updates.

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

[Invalid] The LINK deposit is not checked

This is informational/invalid. If the LINK deposit is not enough, the function `registerUpkeep` will revert and it is responsibility of the user to provide the correct amount of `_initLinkDeposit`, if the user wants automated weather updates.

Support

FAQs

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