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

User Loses Minting Fee if Keeper Registration Attempted with Zero LINK Deposit

willubedaone

Description

  • Normal Behavior: When a user mints an NFT and opts to register a Chainlink Keeper for automated updates, they are expected to provide a non-zero amount of LINK tokens to fund the upkeep. If the LINK deposit is insufficient or zero, the system should ideally reject the request early, preventing the user from spending their minting fee (ETH) on a process that is guaranteed to fail later.

  • Specific Issue: The requestMintWeatherNFT function in WeatherNft.sol allows a user to initiate a mint with _registerKeeper set to true even if _initLinkDeposit is 0. The function does not revert at this stage; instead, it successfully processes the ETH payment, increments the mint price, and initiates a Chainlink Functions request. However, when the fulfillMintRequest function is later called (after the Chainlink Functions response), the attempt to register the keeper via IAutomationRegistrarInterface.registerUpkeep will fail because the amount parameter (derived from _initLinkDeposit) will be zero. This failure causes fulfillMintRequest to revert. Crucially, the ETH paid by the user in the initial requestMintWeatherNFT call is not refunded, and the user does not receive an NFT.

​
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;
​
// @> No check here to ensure _initLinkDeposit > 0 when _registerKeeper is true
if (_registerKeeper) {
// @> This safeTransferFrom(..., 0) call succeeds because transferring 0 tokens is valid
// @> even if the contract has no allowance from msg.sender for LINK, or msg.sender has 0 LINK.
IERC20(s_link).safeTransferFrom(
msg.sender,
address(this),
_initLinkDeposit // This is 0 in the problematic scenario
);
}
​
_reqId = _sendFunctionsWeatherFetchRequest(_pincode, _isoCode);
​
emit WeatherNFTMintRequestSent(msg.sender, _pincode, _isoCode, _reqId);
​
s_funcReqIdToUserMintReq[_reqId] = UserMintRequest({
user: msg.sender,
pincode: _pincode,
isoCode: _isoCode,
registerKeeper: _registerKeeper,
heartbeat: _heartbeat,
initLinkDeposit: _initLinkDeposit // Stored as 0
});
}
​
function fulfillMintRequest(bytes32 requestId) external {
// ...
UserMintRequest memory _userMintRequest = s_funcReqIdToUserMintReq[
requestId
];
// ...
if (_userMintRequest.registerKeeper) { // This is true
// Register chainlink keeper to pull weather data in order to automate weather nft
// @> _userMintRequest.initLinkDeposit is 0, so this approves 0
LinkTokenInterface(s_link).approve(s_keeperRegistrar, _userMintRequest.initLinkDeposit);
​
IAutomationRegistrarInterface.RegistrationParams
memory _keeperParams = IAutomationRegistrarInterface
.RegistrationParams({
// ...
// @> _userMintRequest.initLinkDeposit is 0, so amount is 0
amount: uint96(_userMintRequest.initLinkDeposit)
});
​
// @> This call to IAutomationRegistrarInterface.registerUpkeep will revert
// @> because Chainlink Automation requires a non-zero LINK amount to fund an upkeep.
upkeepId = IAutomationRegistrarInterface(s_keeperRegistrar)
.registerUpkeep(_keeperParams);
}
// ...
}
// ...
​

Risk

Likelihood:

  • When a user mistakenly provides 0 for _initLinkDeposit while intending to register a keeper and believing any LINK on the contract or their balance might be used.

  • When an attacker or curious user probes the minting function with _registerKeeper = true and _initLinkDeposit = 0 to understand system behavior or potentially cause issues.

Impact:

  • Loss of User Funds: The user loses the ETH minting fee paid during the requestMintWeatherNFT call because this transaction succeeds, but the subsequent fulfillMintRequest (which actually mints the NFT and registers the keeper) reverts.

  • No NFT Received: Due to the revert in fulfillMintRequest, the NFT is never minted to the user.

  • Wasted Resources: Gas is consumed for the successful requestMintWeatherNFT call, the Chainlink Functions request (which consumes LINK from the contract's subscription), and the failed fulfillMintRequest call.

Proof of Concept

This test fails with the message [FAIL: next call did not revert as expected]. This directly proves that requestMintWeatherNFT does not revert when _registerKeeper is true and _initLinkDeposit is 0, allowing the problematic sequence to begin.

function testMintRevertsIfRegisterKeeperButZeroDeposit() public {
uint256 mintPrice = weatherNft.s_currentMintPrice();
vm.prank(user);
vm.expectRevert();
weatherNft.requestMintWeatherNFT{value: mintPrice}(
"123456",
"IN",
true,
12 hours,
0
);
}

Recommended Mitigation

Add a requirement in the requestMintWeatherNFT function to ensure that if _registerKeeper is true, then _initLinkDeposit must be greater than zero. This will cause the transaction to revert early, preventing the user from losing their ETH minting fee.

​
+ add this code
s_currentMintPrice += s_stepIncreasePerMint;
​
if (_registerKeeper) {
require(
_initLinkDeposit > 0,
"WeatherNft__ZeroLinkForKeeperRegistration"
);
IERC20(s_link).safeTransferFrom(
msg.sender,
address(this),
Updates

Appeal created

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