Weather Witness

First Flight #40
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Submission Details
Impact: high
Likelihood: medium
Invalid

## WeatherNft.sol ## [ Using _mint can be dangerous ]

Root + Impact

Description

The fulfillMintRequest function uses _mint instead of _safeMint.

When minting NFTs to an address, the _safeMint function is used to ensure that the recipient is capable of handling ERC721 tokens. This includes checking that a smart contract recipient implements the onERC721Received interface, preventing tokens from being permanently locked.

By using_mint function instead of _safeMint. This bypasses the onERC721Received check, which can lead to tokens being locked in the contract that do not support receiving ERC721 tokens.

@> _mint(msg.sender, tokenId);
s_tokenIdToWeather[tokenId] = Weather(weather);

Risk

Likelihood:

Developers often use contract wallets, multisigs, or custom vaults to receive NFTs, which may not support the ERC721 receiver interface.

The function is publicly accessible or accessible via a permissioned interface, making it easy to call with any target address.

Impact:

Tokens can be permanently locked in a smart contract that cannot handle them

This can result in asset loss, affecting users and potentially the reputation of the project

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity 0.8.29;
// @title Mock ERC721 contract demonstrating unsafe vs safe minting
import "@openzeppelin/contracts/token/ERC721/ERC721.sol";
contract VulnerableNFT is ERC721 {
constructor() ERC721("VulnerableNFT", "VNFT") {}
function unsafeMint(address to, uint256 tokenId) public {
_mint(to, tokenId);
}
function safeMint(address to, uint256 tokenId) public {
_safeMint(to, tokenId);
}
}
// @title Mock contract simulating a bad receiver that reverts on NFT receipt
contract BadReceiver {
function onERC721Received(address, address, uint256, bytes calldata)
external pure returns (bytes4) {
revert("BadReceiver: I do not accept NFTs");
}
}
// SPDX-License-Identifier: MIT
pragma solidity 0.8.29;
import {Test, console} from "forge-std/Test.sol";
import {VulnerableNFT, BadReceiver} from "../src/VulnerableNFT.sol";
contract VulnerableNFTTest is Test {
VulnerableNFT vulnerableNFT;
BadReceiver badReceiver;
function setUp() public {
vulnerableNFT = new VulnerableNFT();
badReceiver = new BadReceiver();
}
function test_unsafeMintToBadReceiver_shouldSucceed() public {
vulnerableNFT.unsafeMint(address(badReceiver), 1);
assertEq(vulnerableNFT.ownerOf(1), address(badReceiver));
}
function test_safeMintToBadReceiver_shouldRevert() public {
vm.expectRevert();
vulnerableNFT.safeMint(address(badReceiver), 1);
}
}
Ran 1 test for test/WeatherNftForkTest.t.sol:WeatherNftForkTest
[FAIL: EvmError: Revert] setUp() (gas: 0)
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 55.67ms (0.00ns CPU time)
Ran 2 tests for test/VulnerableNFT.t.sol:VulnerableNFTTest
[PASS] test_safeMintToBadReceiver_shouldRevert() (gas: 61491)
[PASS] test_unsafeMintToBadReceiver_shouldSucceed() (gas: 58523)
Suite result: ok. 2 passed; 0 failed; 0 skipped; finished in 56.26ms (552.10µs CPU time)

Recommended Mitigation

Use _safeMint instead of _mint to prevent tokens from being permanently locked.

- _mint(msg.sender, tokenId);
+ _safeMint(msg.sender, tokenId);
Updates

Appeal created

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