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

Business logic DoS for `WeatherNft::fulfillMintRequest` caused by a lack of check on deposit amount in `WeatherNft::requestMintWeatherNFT` when the user enables the automation (_registerKeeper is set to true).

franzops

Root + Impact

Business logic DoS for WeatherNft::fulfillMintRequest caused by a lack of check on deposit amount in WeatherNft::requestMintWeatherNFT when the user enables the automation (_registerKeeper is set to true).

Description

Business logic DoS for WeatherNft::fulfillMintRequest caused by a lack of check on deposit amount in WeatherNft::requestMintWeatherNFT when the user enables the automation.
This makes the user unable to fulfill their mint request because the deposit amount is not sufficient.

Risk

The requestMintWeatherNFT function does not check if the deposit amount is greater than 0 when the user enables the automation. This can lead to a situation where, each time the user calls fulfillMintRequest, the function reverts because the deposit amount is not sufficient.
There is no function that enables the user to update the deposit amount after the request is sent. This can lead to a situation where the user is unable to fulfill their mint request because the deposit amount is not sufficient.

Likelihood: MEDIUM

  • It's easy to pass 0 value as deposit parameter

Impact: HIGH

  • The request will be stuck and cannot be fulfilled anymore because the deposit amount is not sufficient.

Proof of Concept

function test_fulfillMintRequest_ZeroDeposit() public {
string memory pincode = "125001";
string memory isoCode = "IN";
bool registerKeeper = true;
uint256 heartbeat = 12 hours;
uint256 initLinkDeposit = 0e18;
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));
​
vm.prank(functionsRouter);
bytes memory weatherResponse = abi.encode(WeatherNftStore.Weather.RAINY);
weatherNft.handleOracleFulfillment(reqId, weatherResponse, "");
​
vm.prank(user);
weatherNft.fulfillMintRequest(reqId);
}

Recommended Mitigation

Consider adding a check to ensure that the deposit amount is greater than 0 when the user enables the automation. This can be done by adding a require statement in requestMintWeatherNFT.

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) {
+ require(_initLinkDeposit > 0, "Deposit amount must be greater than 0");
IERC20(s_link).safeTransferFrom(
msg.sender,
address(this),
_initLinkDeposit
);
}
​

Otherwise, consider the possibility of adding a function that allows the user to update the deposit amount after the request is sent.

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.

Support

FAQs

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