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 10 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.

Give us feedback!