DatingDapp

M-04: Reentrancy in mintProfile() lets a contract mint multiple profile NFTs for the same address

Severity: Medium

Summary

SoulboundProfileNFT::mintProfile() calls _safeMint(msg.sender, tokenId) before it updates profileToToken[msg.sender]. If msg.sender is a contract, OpenZeppelin ERC-721’s _safeMint will call onERC721Received on that contract. During that callback, the recipient can reenter mintProfile() while profileToToken[msg.sender] is still 0, allowing multiple NFTs to be minted for the same address. OpenZeppelin documents that _safeMint checks acceptance on contract recipients via onERC721Received, and Solidity documents reentrancy risk whenever external calls happen before state is finalized. ([OpenZeppelin Docs][1])

Vulnerability Details

The vulnerable flow is:

The issue is the ordering:

1. The function checks profileToToken[msg.sender] == 0.

2. It increments _nextTokenId.

3. It calls _safeMint(msg.sender, tokenId).

4. Only after that external interaction does it set:

   • _profiles[tokenId]

   • profileToToken[msg.sender] = tokenId

OpenZeppelin states that _safeMint will call onERC721Received if the recipient is a contract. That callback is an external call boundary. Because the critical “already has a profile” state is not updated until after _safeMint returns, the recipient contract can reenter mintProfile() and pass the same require again. Solidity’s security guidance explicitly warns that external calls before finalizing state can enable reentrancy. ([OpenZeppelin Docs][1])

Impact

A single address can mint multiple profile NFTs even though the protocol expects one profile per address.

This breaks core protocol assumptions:

• profileToToken[address] only tracks the last minted token for that address

• earlier tokens remain owned by the address but are no longer referenced by profileToToken

• downstream logic in LikeRegistry assumes one valid profile per address

• profile burn/block logic may only affect the latest mapped token, leaving extra profile NFTs behind

That makes this more than a cosmetic bug: it breaks identity uniqueness, which is central to the protocol’s design.

Root Cause

The contract violates the checks-effects-interactions pattern by performing _safeMint() before updating the state that enforces uniqueness. Solidity recommends finalizing state changes before external interactions where possible to reduce reentrancy risk. ([Solidity][2])

Internal Preconditions

• SoulboundProfileNFT is deployed

• mintProfile() uses _safeMint

• profileToToken is only updated after _safeMint returns

External Preconditions

• The caller is a contract

• The caller implements onERC721Received

• The callback reenters mintProfile()

Attack Path

1. Attacker deploys a contract implementing onERC721Received.

2. Attacker contract calls mintProfile(...).

3. mintProfile() passes the profileToToken[msg.sender] == 0 check.

4. _safeMint() mints token #1 and calls onERC721Received on the attacker contract.

5. Inside onERC721Received, the attacker reenters mintProfile(...).

6. profileToToken[msg.sender] is still 0, so the second call also passes.

7. A second token is minted for the same address.

8. After unwinding, profileToToken[msg.sender] points only to the latest token, while the same address owns multiple NFTs.

Proof of Concept

PoC Explanation

This test shows that:

• the attacker contract calls mintProfile() once

• _safeMint() triggers onERC721Received

• the callback reenters mintProfile()

• the second mint succeeds because profileToToken[address(attacker)] has not been set yet

• after completion, the same address owns two NFTs, while profileToToken points only to the second one

That proves the one-profile-per-address rule can be bypassed.

Recommendation

Use one of these fixes:

Preferred fix: update state before external interaction
Set the uniqueness marker before calling _safeMint():

Because a revert unwinds state, this remains safe if _safeMint() fails.

Defense-in-depth
Add ReentrancyGuard and mark mintProfile() as nonReentrant.

Alternative
If contract recipients are not needed, use _mint instead of _safeMint, but OpenZeppelin generally recommends _safeMint when possible for ERC-721 recipients. ([OpenZeppelin Docs][1])