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

Loss of LINK Tokens Due to `_initLinkDeposit` Truncation During Keeper Registration

albert

Root + Impact

Description

When a user requests to mint an NFT with automated updates (_registerKeeper = true), they provide _initLinkDeposit as a uint256 value for funding the Chainlink Keeper. This full uint256 amount is transferred from the user to the WeatherNft contract.
However, when this deposit is used to register the upkeep in fulfillMintRequest, the amount field in IAutomationRegistrarInterface.RegistrationParams is a uint96. The contract code casts _userMintRequest.initLinkDeposit (which is uint256) to uint96 using uint96(_userMintRequest.initLinkDeposit).
If the user provides an _initLinkDeposit value larger than type(uint96).max, the value will be truncated. The Chainlink Automation registrar will only pull the truncated uint96 amount of LINK, but the contract has already received the full uint256 amount from the user and approved the full amount to the registrar. The difference (_initLinkDeposit - uint96(_initLinkDeposit)) remains in the WeatherNft contract, approved for the registrar but unutilized by the specific upkeep registration. Since there is no general mechanism to withdraw arbitrary LINK balances from the contract (only specific ones tied to upkeep management, if implemented), this excess LINK becomes permanently locked.

// Root cause in the codebase with @> marks to highlight the relevant section
// File: src/WeatherNft.sol
​
function requestMintWeatherNFT(
// ...
uint256 _heartbeat,
// @> uint256 _initLinkDeposit // User provides a uint256 amount
) external payable returns (bytes32 _reqId) {
// ...
if (_registerKeeper) {
IERC20(s_link).safeTransferFrom( // Full uint256 amount transferred to this contract
msg.sender,
address(this),
_initLinkDeposit
);
}
// ...
s_funcReqIdToUserMintReq[_reqId] = UserMintRequest({
// ...
// @> initLinkDeposit: _initLinkDeposit // Full uint256 amount stored
});
}
​
function fulfillMintRequest(bytes32 requestId) external {
// ...
UserMintRequest memory _userMintRequest = s_funcReqIdToUserMintReq[requestId];
// ...
uint256 upkeepId;
if (_userMintRequest.registerKeeper) {
// Approves the full uint256 amount
LinkTokenInterface(s_link).approve(s_keeperRegistrar, _userMintRequest.initLinkDeposit);
​
IAutomationRegistrarInterface.RegistrationParams
memory _keeperParams = IAutomationRegistrarInterface
.RegistrationParams({
// ...
// @> amount: uint96(_userMintRequest.initLinkDeposit) // uint256 is truncated to uint96 here
});
​
upkeepId = IAutomationRegistrarInterface(s_keeperRegistrar)
.registerUpkeep(_keeperParams); // Registrar only uses the truncated uint96 amount
}
// ...
// If _userMintRequest.initLinkDeposit > type(uint96).max,
// the difference (_userMintRequest.initLinkDeposit - uint96(_userMintRequest.initLinkDeposit))
// remains in this contract's LINK balance, approved to s_keeperRegistrar but not used for this upkeep,
// and effectively locked.
}

Risk

Likelihood: Medium

  • A user might accidentally or intentionally provide a _initLinkDeposit larger than type(uint96).max (approx 79.2 LINK if LINK has 18 decimals). This is not an extremely large sum, making it plausible for a user to deposit, especially if they are unsure about required funding and want to overfund significantly.

  • Front-ends or helper scripts might not validate this limit.

Impact: Medium

  • Permanent Loss of User's LINK: The portion of the LINK deposit exceeding type(uint96).max becomes locked in the contract and is irrecoverable by the user.

  • Reduced Capital Efficiency: User's funds are stuck.

Proof of Concept

  1. Assume type(uint96).max is equivalent to 79 LINK (for simplicity, actual value is 2**96 - 1 wei).

  2. Alice calls requestMintWeatherNFT with _registerKeeper = true and _initLinkDeposit = 100 * 10**18 (100 LINK).

  3. The WeatherNft contract receives 100 LINK from Alice. _userMintRequest.initLinkDeposit is stored as 100 * 10**18.

  4. In fulfillMintRequest, the contract approves s_keeperRegistrar for 100 * 10**18 LINK.

  5. When preparing _keeperParams, amount is set to uint96(100 * 10**18). This truncates to 79 * 10**18 (approx).

  6. registerUpkeep is called. The Chainlink Automation system funds the new upkeep with 79 * 10**18 LINK from the contract's balance (which was approved).

  7. The remaining 100 - 79 = 21 LINK are still held by the WeatherNft contract. These 21 LINK are not associated with Alice's upkeep directly for spending and are not part of any refund or withdrawal mechanism related to this specific upkeep registration's overfunding. They are locked.

Recommended Mitigation

Validate _initLinkDeposit in requestMintWeatherNFT to ensure it does not exceed type(uint96).max. If it does, either revert the transaction or cap the deposit at type(uint96).max. Reverting is safer to ensure user intent is clear.

// File: src/WeatherNft.sol
// At the top, if not already present for other reasons:
// import {Type} from "@openzeppelin/contracts/utils/math/SafeCast.sol"; // Or just use type(uint96).max
​
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 increment moved to fulfillMintRequest (as per other recommendation)
​
if (_registerKeeper) {
+ require(_initLinkDeposit <= type(uint96).max, "WeatherNft__LinkDepositExceedsMax");
IERC20(s_link).safeTransferFrom(
msg.sender,
address(this),
_initLinkDeposit
);
}
​
_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 // Now guaranteed to be <= type(uint96).max if _registerKeeper is true
});
}
​
// No change needed in fulfillMintRequest for this specific issue if the above require is added,
// as _userMintRequest.initLinkDeposit will already be within uint96 range.
function fulfillMintRequest(bytes32 requestId) external {
// ...
UserMintRequest memory _userMintRequest = s_funcReqIdToUserMintReq[requestId];
// ...
uint256 upkeepId;
if (_userMintRequest.registerKeeper) {
LinkTokenInterface(s_link).approve(s_keeperRegistrar, _userMintRequest.initLinkDeposit);
​
IAutomationRegistrarInterface.RegistrationParams
memory _keeperParams = IAutomationRegistrarInterface
.RegistrationParams({
// ...
// _userMintRequest.initLinkDeposit is now safe to cast to uint96 without truncation loss
// because of the check in requestMintWeatherNFT.
amount: uint96(_userMintRequest.initLinkDeposit)
});
​
upkeepId = IAutomationRegistrarInterface(s_keeperRegistrar)
.registerUpkeep(_keeperParams);
}
// ...
}
Updates

Appeal created

bube Lead Judge 5 months ago
Submission Judgement Published
Validated
Assigned finding tags:

[Invalid] Loss of LINK tokens

After discussion with the sponsor, this turns out to be invalid. This is because the Chainlink has a capped maximum supply of 1 billion LINK tokens. This means that the total number of LINK tokens will never exceed 1 billion. The token has 18 decimals, so the max scaled value that is required to represent all LINK tokens is 1e27. The max value of `uint96` is 2**96 - 1, that is around 79e27 and it is sufficient to store all LINK tokens. Therefore, the cast from uint256 to uint96 is safe and there is no possibility of token truncation/loss of tokens.

bube Lead Judge 5 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

[Invalid] Loss of LINK tokens

After discussion with the sponsor, this turns out to be invalid. This is because the Chainlink has a capped maximum supply of 1 billion LINK tokens. This means that the total number of LINK tokens will never exceed 1 billion. The token has 18 decimals, so the max scaled value that is required to represent all LINK tokens is 1e27. The max value of `uint96` is 2**96 - 1, that is around 79e27 and it is sufficient to store all LINK tokens. Therefore, the cast from uint256 to uint96 is safe and there is no possibility of token truncation/loss of tokens.

Support

FAQs

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