Weather Witness

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

No check on approve() return in fulfillmentMintRequest()

Root + Impact

Description

  • There is no check of the return when approving the contract to pull from the user a Link deposit for automation

    with Chainlink of the weather update

  • Usually, approve() function returns if succesfull or not

  • Here this is not checked

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) {
LinkTokenInterface(s_link).approve(
s_keeperRegistrar,
_userMintRequest.initLinkDeposit
); // @audit-issue no check on approve return
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
});
}

Risk

Likelihood:

  • Reason 1 Should not happen usually, but it can under special circumstances, invalid input, contract restrictions, or gas issues.

  • Reason 2

Impact:

  • Impact 1 Will stop execution, with no feedback since there is no error emited

  • Impact 2 Will cause a severe disruption of protocol functionality

Proof of Concept

Recommended Mitigation

Add a boolean and an "if" check on the return of approve.
This will show why the approve failed.

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) {
bool succes = LinkTokenInterface(s_link).approve(
s_keeperRegistrar,
_userMintRequest.initLinkDeposit
); // @audit-issue no check on approve return
// + if (!success) {
// + return "Aprove failed";
}
// ...Rest of code
}
Updates

Appeal created

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

Support

FAQs

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