Beginner FriendlyFoundryNFT
100 EXP
View results
Submission Details
Severity: medium
Valid

A user calling `Soulmate::mintSoulmateToken` two times in a row one after another will make them their own soulmate

Summary

The Soulmate::mintSoulmateToken function allows users to get assigned to soulmates or wait for a soulmate, but considering the scenario where there is no user waiting for a soulmate and a user places the request, then he will be placed in the waiting list untill another persons calls the function, but if the same person again calls it then the user ends up being their own soulmate.
A user can thus become their own soulmate if they call the function twice in quick succession.

Vulnerability Details

The vulnerability is present in the Soulmate::mintSoulmateToken function where if a person who has called it once and waiting for another user to call and become their soulmate calls it again by mistake or by intentionally will make them their own soulmate.
This occurs due to missing address check to prevent the user waiting for soulmate to prevent calling the function again so that they do not become their own soulmate.
The first call to mintSoulmateToken would set the user's address inside idToOwners corresponding to the current nextID at 0th idx. If the user calls the function again before another address, they would be set as their own soulmate as the function does not check whether the second soulmate is the same as the first.

Impact

The impact of this vulnerability is that it violates the intended functionality of the Soulmate protocol, which is to create pairs of soulmates. By allowing a user to become their own soulmate, the contract fails to maintain the integrity of the soulmate pairings.

Tools Used

Manual Review, Unit Test in Foundry

PoC

Add the test in the file: test/unit/SoulmateTest.t.sol

Run the test:

forge test --mt test_SamePersonCanBecomeTheirOwnSoulmate
function test_SamePersonCanBecomeTheirOwnSoulmate() public {
vm.startPrank(soulmate1);
// soulmate1 wants to find a soulmate
soulmateContract.mintSoulmateToken();
// soulmate1 again calls function, and get paired with themselves
soulmateContract.mintSoulmateToken();
vm.stopPrank();
// soulmate1 end up being their own soulmate
assertEq(soulmateContract.soulmateOf(soulmate1), soulmate1);
}

Recommendations

Add a check within the function to verify that the second soulmate is not the same as the first before proceeding with the minting process.

+ error Soulmate__AlreadyInWaiting();
function mintSoulmateToken() public returns (uint256) {
// Check if people already have a soulmate, which means already have a token
address soulmate = soulmateOf[msg.sender];
if (soulmate != address(0))
revert Soulmate__alreadyHaveASoulmate(soulmate);
address soulmate1 = idToOwners[nextID][0];
address soulmate2 = idToOwners[nextID][1];
+ if (msg.sender == soulmate1) {
+ revert Soulmate__AlreadyInWaiting();
+ }
if (soulmate1 == address(0)) {
idToOwners[nextID][0] = msg.sender;
ownerToId[msg.sender] = nextID;
emit SoulmateIsWaiting(msg.sender);
} else if (soulmate2 == address(0)) {
idToOwners[nextID][1] = msg.sender;
// Once 2 soulmates are reunited, the token is minted
ownerToId[msg.sender] = nextID;
soulmateOf[msg.sender] = soulmate1;
soulmateOf[soulmate1] = msg.sender;
idToCreationTimestamp[nextID] = block.timestamp;
emit SoulmateAreReunited(soulmate1, soulmate2, nextID);
_mint(msg.sender, nextID++);
}
return ownerToId[msg.sender];
}
Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-self-soulmate

- Given the native anonymous nature of blockchain in general, this issue cannot be avoided unless an explicit whitelist is implemented. Even then we can only confirm soulmates are distinct individuals via kyc. I believe finding a soulmate is intended to be permisionless. - However, even though sufficient (500_000_000e18 in each vault) tokens are minted to claim staking and airdrop rewards, it would take 500_000_000 / 2 combined weeks for airdrop vault to be drained which is not unreasonable given there are [80+ million existing wallets](https://coinweb.com/trends/how-many-crypto-wallets-are-there/). Given there is no option to mint new love tokens, this would actually ruin the functionality of the protocol of finding soulmates and shift the focus to abusing a sybil attack to farming airdrops instead. Assigning medium severity for now but am open for appeals otherwise, since most if not all issues lack indepth analysis of the issue.

Support

FAQs

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