Weather Witness
First Flight #40
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Impact: medium
Likelihood: low
Invalid

The `WeatherNft::fulfillMintRequest` function uses `_mint`, which omits recipient contract safety checks

arinthal

Root + Impact

Description

The WeatherNft::fulfillMintRequest function mints an NFT based on weather conditions at a specified location.

It calls the low-level _mint function directly instead of the safer _safeMint variant. This approach is discouraged because _mint does not verify whether the recipient address can properly handle ERC721 tokens. If the recipient is a smart contract that does not implement the IERC721Receiver interface, the NFT may become permanently locked within that contract.

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;
}
​
UserMintRequest memory _userMintRequest = s_funcReqIdToUserMintReq[
requestId
];
uint8 weather = abi.decode(response, (uint8));
uint256 tokenId = s_tokenCounter;
s_tokenCounter++;
​
emit WeatherNFTMinted(
requestId,
msg.sender,
Weather(weather)
);
@> _mint(msg.sender, tokenId);
s_tokenIdToWeather[tokenId] = Weather(weather);
​
​
​
// rest of the function code
​
​
}

Risk

Likelihood:

  • The problem arises when the token is minted by a contract that does not handle its transfer correctly.

Impact:

  • Potential for NFTs to be irretrievably locked if minted to a contract that does not properly handle ERC721 transfers.

Recommended Mitigation

Use the _safeMint function instead of _mint to ensure that the recipient address can safely handle ERC721 tokens.

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;
}
​
UserMintRequest memory _userMintRequest = s_funcReqIdToUserMintReq[
requestId
];
uint8 weather = abi.decode(response, (uint8));
uint256 tokenId = s_tokenCounter;
s_tokenCounter++;
​
emit WeatherNFTMinted(
requestId,
msg.sender,
Weather(weather)
);
- _mint(msg.sender, tokenId);
+ _safeMint(msg.sender, tokenId);
s_tokenIdToWeather[tokenId] = Weather(weather);
​
​
// rest of the function code
​
}
​

The _safeMint function performs an additional check by calling onERC721Received on the recipient contract to confirm that it can handle ERC721 tokens.

Updates

Appeal created

bube Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

[Invalid] Use of `_mint` istead of `_safeMint`

The `fulfillMintRequest` function is external and anyone can call it. If the protocol uses `_safeMint` instead of `_mint`, this introduces a reentrancy risk. It is better to use `_mint` and the caller is responsible for being able to obtain the token.

Support

FAQs

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