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

NFT Minted to Incorrect User (Front-Runner) Due to `msg.sender` Misuse in `fulfillMintRequest`

albert

Root + Impact

Description

The NFT minting process is a two-step operation:

  1. A user calls requestMintWeatherNFT to initiate the mint, paying a fee and optionally depositing LINK for automation. This function stores the original minter's address (_userMintRequest.user).

  2. After the Chainlink oracle responds, the user (or anyone) calls fulfillMintRequest to finalize the minting and receive the NFT.

The issue is that fulfillMintRequest mints the NFT to msg.sender (the caller of fulfillMintRequest) instead of the original user who initiated the request (_userMintRequest.user). This allows a front-runner, observing the oracle's response transaction, to call fulfillMintRequest before the legitimate minter, thereby stealing the NFT for which the original minter paid the minting fee and (if applicable) deposited LINK for automation.

// Root cause in the codebase with @> marks to highlight the relevant section
// File: src/WeatherNft.sol
​
function requestMintWeatherNFT(
string memory _pincode,
string memory _isoCode,
bool _registerKeeper,
uint256 _heartbeat,
uint256 _initLinkDeposit
) external payable returns (bytes32 _reqId) {
// ...
s_funcReqIdToUserMintReq[_reqId] = UserMintRequest({
// @> user: msg.sender, // Original minter stored here
pincode: _pincode,
isoCode: _isoCode,
registerKeeper: _registerKeeper,
heartbeat: _heartbeat,
initLinkDeposit: _initLinkDeposit
});
}
​
function fulfillMintRequest(bytes32 requestId) external {
// ...
UserMintRequest memory _userMintRequest = s_funcReqIdToUserMintReq[
requestId
];
uint8 weather = abi.decode(response, (uint8));
uint256 tokenId = s_tokenCounter;
s_tokenCounter++;
​
emit WeatherNFTMinted(
requestId,
// @> msg.sender, // Event emits the front-runner's address as user
Weather(weather)
);
// @> _mint(msg.sender, tokenId); // NFT is minted to msg.sender (caller of fulfillMintRequest), not _userMintRequest.user
s_tokenIdToWeather[tokenId] = Weather(weather);
​
// ... Keeper registration logic ...
}

Risk

Likelihood: High

  • The fulfillMintRequest function is external and permissionless once the oracle has fulfilled the initial request.

  • Transactions calling fulfillMintRequest can be observed in the mempool by front-running bots, which can then submit their own transaction with a higher gas price to get their call executed first.

Impact: High

  • NFT Theft: The original user who paid the mint fee and (if applicable) deposited LINK for automation services loses the NFT to the front-runner.

  • Loss of Funds for Original Minter: The original minter pays the mint price and potentially deposits LINK, but does not receive the NFT if front-run. These funds are not refunded.

Proof of Concept

  1. Alice calls requestMintWeatherNFT paying s_currentMintPrice and _initLinkDeposit. s_funcReqIdToUserMintReq[reqId].user is set to Alice's address.

  2. The Chainlink Oracle calls handleOracleFulfillment which in turn calls fulfillRequest, populating s_funcReqIdToMintFunctionReqResponse[reqId].

  3. Bob (a front-runner) sees the oracle transaction in the mempool. Bob immediately calls fulfillMintRequest(reqId) with a higher gas price than Alice's potential call.

  4. Bob's call to fulfillMintRequest executes first. The NFT is minted to Bob (_mint(msg.sender, tokenId) where msg.sender is Bob).

  5. Alice attempts to call fulfillMintRequest but it may fail (e.g., if s_tokenCounter logic prevents reminting or if state is cleaned up, though currently it's not cleaned). Even if her call succeeds, Bob already owns the NFT associated with her initial request parameters. The primary issue is Bob gets the NFT.

Recommended Mitigation

Modify fulfillMintRequest to mint the NFT to the user stored in the UserMintRequest struct, who is the original initiator of the mint request. Also, update the WeatherNFTMinted event to reflect the correct user.

// File: src/WeatherNft.sol
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) {
// Consider refunding or allowing retry if oracle errored.
// For now, at least ensure state is cleaned if aborting.
// delete s_funcReqIdToUserMintReq[requestId]; // Add cleanup
// delete s_funcReqIdToMintFunctionReqResponse[requestId]; // Add cleanup
return;
}
​
UserMintRequest memory _userMintRequest = s_funcReqIdToUserMintReq[
requestId
];
// Add a check to ensure this is a valid, pending mint request for a known user
require(_userMintRequest.user != address(0), "WeatherNft__InvalidRequest");
​
uint8 weather = abi.decode(response, (uint8));
uint256 tokenId = s_tokenCounter;
s_tokenCounter++;
​
emit WeatherNFTMinted(
requestId,
- msg.sender,
+ _userMintRequest.user,
Weather(weather)
);
- _mint(msg.sender, tokenId);
+ _mint(_userMintRequest.user, tokenId);
s_tokenIdToWeather[tokenId] = Weather(weather);
​
uint256 upkeepId;
if (_userMintRequest.registerKeeper) {
// Register chainlink keeper to pull weather data in order to automate weather nft
LinkTokenInterface(s_link).approve(s_keeperRegistrar, _userMintRequest.initLinkDeposit);
​
IAutomationRegistrarInterface.RegistrationParams
memory _keeperParams = IAutomationRegistrarInterface
.RegistrationParams({
name: string.concat(
"Weather NFT Keeper: ",
Strings.toString(tokenId)
),
encryptedEmail: "",
upkeepContract: address(this),
gasLimit: s_upkeepGaslimit,
adminAddress: address(this), // Consider who should be admin
triggerType: 0,
checkData: abi.encode(tokenId),
triggerConfig: "",
offchainConfig: "",
amount: uint96(_userMintRequest.initLinkDeposit)
});
​
upkeepId = IAutomationRegistrarInterface(s_keeperRegistrar)
.registerUpkeep(_keeperParams);
}
​
s_weatherNftInfo[tokenId] = WeatherNftInfo({
heartbeat: _userMintRequest.heartbeat,
lastFulfilledAt: block.timestamp,
upkeepId: upkeepId,
pincode: _userMintRequest.pincode,
isoCode: _userMintRequest.isoCode
});
​
// Clean up state for the completed/failed mint request
delete s_funcReqIdToUserMintReq[requestId];
delete s_funcReqIdToMintFunctionReqResponse[requestId];
}
Updates

Appeal created

bube Lead Judge 6 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.