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

Conversion - loss of precision for link deposit

accessdenied

Root + Impact

`initLinkDeposit, a uint256,is converted to an uint96 in FullFillMintRequest.

There is a potentiel of incorrect recording of deposit funds if the user put an amount > 2^96 as a deposit when he creates is NFT


Description

When creating the parameters for the keeper, the value of initLinkDeposit which represents the initial link deposit by the user during the mint is converted to a uint96 while the current unit is uint256.

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: "",
// @audit : uint256 - uint96 conversion
amount: uint96(_userMintRequest.initLinkDeposit)
});
​

Risk

Likelihood:

the user makes an initial deposit > 2^96 (low)

Impact:

The check will depend of how this amount is used by the keeper, but it is probably to know the amount deposited by the user.

Proof of Concept

Use a deposit > 2^96

​

Recommended Mitigation

Use an uint96 for the function parameter in requestMintWeather NFT and in the corresponding struct UserMintRequest

- remove this code
function requestMintWeatherNFT(
[...]
uint256 _initLinkDeposit
) external payable returns (bytes32 _reqId)
​
​
struct UserMintRequest {
[...]
uint256 initLinkDeposit;
string pincode;
string isoCode;
}
+ add this code
function requestMintWeatherNFT(
[...]
uint96 _initLinkDeposit
) external payable returns (bytes32 _reqId)
​
struct UserMintRequest {
[...]
uint96 initLinkDeposit;
string pincode;
string isoCode;
}
Updates

Appeal created

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