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

Use of `_mint` Instead of `safeMint` Breaks Core Transferability and Can Result in Irrecoverable NFTs in `WeatherNft::fulfillMintRequest`

okhayeelizabeth

Root + Impact

Description

The Weather NFT contract calls _mint directly during minting.
This approach bypasses the critical ERC721Receiver check, meaning if msg.sender is a smart contract that does not implement onERC721Received, the NFT is minted but stuck, it cannot be interacted with, nor transferred, and is effectively burned without intention.

This directly violates the application’s core functional expectation:
Users who mint and own Weather NFTs should be able to transfer ownership

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)
);
//@audit uses _mint instead of safeMInt...
@> _mint(msg.sender, tokenId);
s_tokenIdToWeather[tokenId] = Weather(weather);

Risk

Likelihood: low

  • It requires a smart contract that can not handle ERC721 token

Impact: High

  • Irrecoverable user assets: Minted NFTs sent to smart contracts are permanently inaccessible.

  • Violation of ERC721 compliance: safeMint is the standard approach to prevent such breakages.

Proof of Concept

​

Recommended Mitigation

Prefer using _safeMint over _mint for ERC721 tokens, but do this very carefully, because this opens up a reentrancy attack vector. It's best to add a nonReentrant modifier in the method that is calling _safeMint because of this.

_ function fulfillMintRequest(bytes32 requestId) external {
+ function fulfillMintRequest(bytes32 requestId) external nonReentrant{
//rest of the code ...
​
@L158
- _mint(msg.sender, tokenId);
+ _safeMint(msg.sender, tokenId);
Updates

Appeal created

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