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

Incorrect Conditional Logic Prevents `fulfillWeatherUpdate()` From Being Called After Minting

0xshield

Root + Impact

Description

The fulfillRequest function plays a central role in processing Chainlink Functions responses. It is supposed to handle two separate cases:

  1. If the requestId is associated with a mint query, it stores the response in funcReqIdToMintFunctionReqResponse.

  2. If the requestId is linked to a weather update, it immediately calls _fulfillWeatherUpdate to update the NFT metadata.

However, the implemented conditional logic systematically prevents the execution of the second case when both types of data are present for the same requestId.

if (s_funcReqIdToUserMintReq[requestId].user != address(0)) {
...
}
else if (s_funcReqIdToTokenIdUpdate[requestId] > 0) {
//@> This is never reached when mint data exists, even if update data also exists.
_fulfillWeatherUpdate(...);
}
​
* `s_funcReqIdToUserMintReq`: contains the address of the user who made the mint.
* `funcs_ReqIdToTokenIdUpdate`: Associates the same `requestId` with the `tokenId` to be updated.
​
However, as `s_funcReqIdToUserMintReq[requestId]. user!= address(0)` is always true in this context, the block `else if` is **systematically ignored**, thus preventing `_fulfillment WeatherUpdate()`from being called.
​
This behavior results in a absence of weather update when it is expected. This constitutes a break in logic for the user who has paid the mint with automation.
​
## Risk
**Likelihood**:
​
This issue occurs every time a user requests a mint with registerKeeper = true, since both mappings are populated.
​
The faulty logic prevents _fulfillWeatherUpdate() from ever being triggered in these cases.
​
**Impact**:
​
Automation-based weather updates never occur, breaking a core feature.
​
Users spend LINK expecting automation, but no automated updates are performed.
​
## Proof of Concept
Add the following test to `WeatherNftForkTest.t.sol`:
​
<details>
<summary>Click to view PoC</summary>
​
```solidity
contract WeatherNftForkTest is Test, WeatherNftStore
^^^^^^^^^^^^^^^

And

function testFulfillWeatherUpdateIsCalled() public {
string memory pincode = "125001";
string memory isoCode = "IN";
bool registerKeeper = true;
uint256 heartbeat = 12 hours;
uint256 initLinkDeposit = 5e18;
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));
​
(
uint256 reqHeartbeat,
address reqUser,
bool reqRegisterKeeper,
uint256 reqInitLinkDeposit,
string memory reqPincode,
string memory reqIsoCode
) = weatherNft.s_funcReqIdToUserMintReq(reqId);
assertEq(reqUser, user);
assertEq(reqHeartbeat, heartbeat);
assertEq(reqInitLinkDeposit, initLinkDeposit);
assertEq(reqIsoCode, isoCode);
assertEq(reqPincode, pincode);
assertEq(reqRegisterKeeper, registerKeeper);
vm.expectEmit(true, true, false, false);
emit NftWeatherUpdated(tokenId, WeatherNftStore.Weather.SNOW);
​
vm.prank(functionsRouter);
bytes memory weatherResponse = abi.encode(WeatherNftStore.Weather.SNOW);
weatherNft.handleOracleFulfillment(reqId, weatherResponse, "");
}

This test fails because NftWeatherUpdated is not emitted, confirming that _fulfillWeatherUpdate() was never invoked — even though a valid tokenId existed in s_funcReqIdToTokenIdUpdate.

Recommended Mitigation

Refactor the logic to allow both cases to be executed independently. One safe approach is to split the two conditions instead of using if / else if:

​
function fulfillRequest(
bytes32 requestId,
bytes memory response,
bytes memory err
) internal override {
if (s_funcReqIdToUserMintReq[requestId].user != address(0)) {
s_funcReqIdToMintFunctionReqResponse[requestId] = MintFunctionReqResponse({
response: response,
err: err
});
}
​
+ if (s_funcReqIdToTokenIdUpdate[requestId] > 0) {
+ _fulfillWeatherUpdate(requestId, response, err);
+ }
- else if (s_funcReqIdToTokenIdUpdate[requestId] > 0) {
- _fulfillWeatherUpdate(requestId, response, err);
- }
​
}
​

This way, both actions are executed when appropriate.

Updates

Appeal created

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

[Invalid] The weather is not updated in `fulfillRequest`

Actually this is not correct. The `requestId` for the `s_funcReqIdToUserMintReq` is created in `requestMintWeatherNFT` after sending a request to the Chainlink. The `requestId` for the `s_funcReqIdToTokenIdUpdate` mapping is created in `performUpkeep` function after sending a request to the Chainlink. The Chainlink returns always unique `requestIds`. The `fulfillRequest` accepts as argument one `requestId` and there is no chance that the both conditions in the `fulfillRequest` function will be `true` at the same time, if the `requestId` is from the `s_funcReqIdToUserMintReq` mapping, it can't be from the `s_funcReqIdToTokenIdUpdate` and vice versa.

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

[Invalid] The weather is not updated in `fulfillRequest`

Actually this is not correct. The `requestId` for the `s_funcReqIdToUserMintReq` is created in `requestMintWeatherNFT` after sending a request to the Chainlink. The `requestId` for the `s_funcReqIdToTokenIdUpdate` mapping is created in `performUpkeep` function after sending a request to the Chainlink. The Chainlink returns always unique `requestIds`. The `fulfillRequest` accepts as argument one `requestId` and there is no chance that the both conditions in the `fulfillRequest` function will be `true` at the same time, if the `requestId` is from the `s_funcReqIdToUserMintReq` mapping, it can't be from the `s_funcReqIdToTokenIdUpdate` and vice versa.

Support

FAQs

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